From adfd853489ed0b76b21d628f1fe479d797ce572f Mon Sep 17 00:00:00 2001 From: Ashley Dumaine Date: Wed, 4 Dec 2024 14:50:03 -0500 Subject: [PATCH] create new FirewallRule CRD --- PROJECT | 8 + Tiltfile | 1 + api/v1alpha2/firewallrule_types.go | 83 +++++++++ api/v1alpha2/linodefirewall_types.go | 36 ++-- api/v1alpha2/zz_generated.deepcopy.go | 106 ++++++++++- ...ucture.cluster.x-k8s.io_firewallrules.yaml | 143 +++++++++++++++ ...ture.cluster.x-k8s.io_linodefirewalls.yaml | 106 +++++++++++ config/crd/kustomization.yaml | 3 + .../patches/cainjection_in_firewallrules.yaml | 7 + .../crd/patches/webhook_in_firewallrules.yaml | 16 ++ config/rbac/firewallrule_editor_role.yaml | 31 ++++ config/rbac/firewallrule_viewer_role.yaml | 27 +++ config/rbac/role.yaml | 2 + .../infrastructure_v1alpha2_firewallrule.yaml | 12 ++ config/samples/kustomization.yaml | 1 + docs/src/topics/firewalling.md | 57 ++++-- .../assert-capi-resources.yaml | 15 ++ .../assert-fwrule-firewall.yaml | 27 +++ .../chainsaw-test.yaml | 88 +++++++++ .../check-fwrule-firewall-deletion.yaml | 25 +++ .../create-fwrule-firewall.yaml | 25 +++ go.mod | 2 +- .../controller/linodefirewall_controller.go | 58 +++++- .../linodefirewall_controller_helpers.go | 172 ++++++++++-------- .../linodefirewall_controller_helpers_test.go | 41 +++-- .../linodefirewall_controller_test.go | 41 ++++- 26 files changed, 978 insertions(+), 155 deletions(-) create mode 100644 api/v1alpha2/firewallrule_types.go create mode 100644 config/crd/bases/infrastructure.cluster.x-k8s.io_firewallrules.yaml create mode 100644 config/crd/patches/cainjection_in_firewallrules.yaml create mode 100644 config/crd/patches/webhook_in_firewallrules.yaml create mode 100644 config/rbac/firewallrule_editor_role.yaml create mode 100644 config/rbac/firewallrule_viewer_role.yaml create mode 100644 config/samples/infrastructure_v1alpha2_firewallrule.yaml create mode 100644 e2e/linodefirewall-controller/linodefirewall-firewall-rule/assert-capi-resources.yaml create mode 100644 e2e/linodefirewall-controller/linodefirewall-firewall-rule/assert-fwrule-firewall.yaml create mode 100644 e2e/linodefirewall-controller/linodefirewall-firewall-rule/chainsaw-test.yaml create mode 100644 e2e/linodefirewall-controller/linodefirewall-firewall-rule/check-fwrule-firewall-deletion.yaml create mode 100644 e2e/linodefirewall-controller/linodefirewall-firewall-rule/create-fwrule-firewall.yaml diff --git a/PROJECT b/PROJECT index cda708947..f2c00d410 100644 --- a/PROJECT +++ b/PROJECT @@ -121,4 +121,12 @@ resources: kind: AddressSet path: github.com/linode/cluster-api-provider-linode/api/v1alpha2 version: v1alpha2 +- api: + crdVersion: v1 + namespaced: true + domain: cluster.x-k8s.io + group: infrastructure + kind: FirewallRule + path: github.com/linode/cluster-api-provider-linode/api/v1alpha2 + version: v1alpha2 version: "3" diff --git a/Tiltfile b/Tiltfile index 4664f6316..6d628c712 100644 --- a/Tiltfile +++ b/Tiltfile @@ -100,6 +100,7 @@ if os.getenv("INSTALL_RKE2_PROVIDER", "false") == "true": capl_resources = [ "capl-system:namespace", "addresssets.infrastructure.cluster.x-k8s.io:customresourcedefinition", + "firewallrules.infrastructure.cluster.x-k8s.io:customresourcedefinition", "linodeclusters.infrastructure.cluster.x-k8s.io:customresourcedefinition", "linodemachines.infrastructure.cluster.x-k8s.io:customresourcedefinition", "linodeclustertemplates.infrastructure.cluster.x-k8s.io:customresourcedefinition", diff --git a/api/v1alpha2/firewallrule_types.go b/api/v1alpha2/firewallrule_types.go new file mode 100644 index 000000000..a06e25a84 --- /dev/null +++ b/api/v1alpha2/firewallrule_types.go @@ -0,0 +1,83 @@ +/* +Copyright 2023 Akamai Technologies, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha2 + +import ( + "github.com/linode/linodego" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! +// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. + +// FirewallRuleSpec defines the desired state of FirewallRule +type FirewallRuleSpec struct { + // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster + // Important: Run "make" to regenerate code after modifying this file + Action string `json:"action"` + Label string `json:"label"` + Description string `json:"description,omitempty"` + Ports string `json:"ports,omitempty"` + // +kubebuilder:validation:Enum=TCP;UDP;ICMP;IPENCAP + Protocol linodego.NetworkProtocol `json:"protocol"` + Addresses *NetworkAddresses `json:"addresses,omitempty"` + // AddressSetRefs is a list of references to AddressSets as an alternative to + // using Addresses but can be used in conjunction with it + AddressSetRefs []*corev1.ObjectReference `json:"addressSetRefs,omitempty"` +} + +// NetworkAddresses holds a list of IPv4 and IPv6 addresses +// We don't use linodego here since kubebuilder can't generate DeepCopyInto +// for linodego.NetworkAddresses +type NetworkAddresses struct { + IPv4 *[]string `json:"ipv4,omitempty"` + IPv6 *[]string `json:"ipv6,omitempty"` +} + +// FirewallRuleStatus defines the observed state of FirewallRule +type FirewallRuleStatus struct { + // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster + // Important: Run "make" to regenerate code after modifying this file +} + +//+kubebuilder:object:root=true +//+kubebuilder:resource:path=firewallrules,scope=Namespaced,categories=cluster-api,shortName=fwr +//+kubebuilder:subresource:status +// +kubebuilder:metadata:labels="clusterctl.cluster.x-k8s.io/move-hierarchy=true" + +// FirewallRule is the Schema for the firewallrules API +type FirewallRule struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec FirewallRuleSpec `json:"spec,omitempty"` + Status FirewallRuleStatus `json:"status,omitempty"` +} + +//+kubebuilder:object:root=true + +// FirewallRuleList contains a list of FirewallRule +type FirewallRuleList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []FirewallRule `json:"items"` +} + +func init() { + SchemeBuilder.Register(&FirewallRule{}, &FirewallRuleList{}) +} diff --git a/api/v1alpha2/linodefirewall_types.go b/api/v1alpha2/linodefirewall_types.go index 776fe58fa..5204a17b0 100644 --- a/api/v1alpha2/linodefirewall_types.go +++ b/api/v1alpha2/linodefirewall_types.go @@ -17,7 +17,6 @@ limitations under the License. package v1alpha2 import ( - "github.com/linode/linodego" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -39,7 +38,12 @@ type LinodeFirewallSpec struct { Enabled bool `json:"enabled,omitempty"` // +optional - InboundRules []FirewallRule `json:"inboundRules,omitempty"` + InboundRules []FirewallRuleSpec `json:"inboundRules,omitempty"` + + // InboundRuleRefs is a list of references to FirewallRules as an alternative to + // using InboundRules but can be used in conjunction with it + // +optional + InboundRuleRefs []*corev1.ObjectReference `json:"inboundRuleRefs,omitempty"` // InboundPolicy determines if traffic by default should be ACCEPTed or DROPped. Defaults to ACCEPT if not defined. // +kubebuilder:validation:Enum=ACCEPT;DROP @@ -48,7 +52,12 @@ type LinodeFirewallSpec struct { InboundPolicy string `json:"inboundPolicy,omitempty"` // +optional - OutboundRules []FirewallRule `json:"outboundRules,omitempty"` + OutboundRules []FirewallRuleSpec `json:"outboundRules,omitempty"` + + // OutboundRuleRefs is a list of references to FirewallRules as an alternative to + // using OutboundRules but can be used in conjunction with it + // +optional + OutboundRuleRefs []*corev1.ObjectReference `json:"outboundRuleRefs,omitempty"` // OutboundPolicy determines if traffic by default should be ACCEPTed or DROPped. Defaults to ACCEPT if not defined. // +kubebuilder:validation:Enum=ACCEPT;DROP @@ -62,27 +71,6 @@ type LinodeFirewallSpec struct { CredentialsRef *corev1.SecretReference `json:"credentialsRef,omitempty"` } -type FirewallRule struct { - Action string `json:"action"` - Label string `json:"label"` - Description string `json:"description,omitempty"` - Ports string `json:"ports,omitempty"` - // +kubebuilder:validation:Enum=TCP;UDP;ICMP;IPENCAP - Protocol linodego.NetworkProtocol `json:"protocol"` - Addresses *NetworkAddresses `json:"addresses,omitempty"` - // AddressSetRefs is a list of references to AddressSets as an alternative to - // using Addresses but can be used in conjunction with it - AddressSetRefs []*corev1.ObjectReference `json:"addressSetRefs,omitempty"` -} - -// NetworkAddresses holds a list of IPv4 and IPv6 addresses -// We don't use linodego here since kubebuilder can't generate DeepCopyInto -// for linodego.NetworkAddresses -type NetworkAddresses struct { - IPv4 *[]string `json:"ipv4,omitempty"` - IPv6 *[]string `json:"ipv6,omitempty"` -} - // LinodeFirewallStatus defines the observed state of LinodeFirewall type LinodeFirewallStatus struct { // Ready is true when the provider resource is ready. diff --git a/api/v1alpha2/zz_generated.deepcopy.go b/api/v1alpha2/zz_generated.deepcopy.go index 21dff695d..16f2dbebd 100644 --- a/api/v1alpha2/zz_generated.deepcopy.go +++ b/api/v1alpha2/zz_generated.deepcopy.go @@ -152,6 +152,65 @@ func (in *BucketAccessRef) DeepCopy() *BucketAccessRef { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *FirewallRule) DeepCopyInto(out *FirewallRule) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + out.Status = in.Status +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FirewallRule. +func (in *FirewallRule) DeepCopy() *FirewallRule { + if in == nil { + return nil + } + out := new(FirewallRule) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *FirewallRule) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *FirewallRuleList) DeepCopyInto(out *FirewallRuleList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]FirewallRule, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FirewallRuleList. +func (in *FirewallRuleList) DeepCopy() *FirewallRuleList { + if in == nil { + return nil + } + out := new(FirewallRuleList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *FirewallRuleList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *FirewallRuleSpec) DeepCopyInto(out *FirewallRuleSpec) { *out = *in if in.Addresses != nil { in, out := &in.Addresses, &out.Addresses @@ -171,12 +230,27 @@ func (in *FirewallRule) DeepCopyInto(out *FirewallRule) { } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FirewallRule. -func (in *FirewallRule) DeepCopy() *FirewallRule { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FirewallRuleSpec. +func (in *FirewallRuleSpec) DeepCopy() *FirewallRuleSpec { if in == nil { return nil } - out := new(FirewallRule) + out := new(FirewallRuleSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *FirewallRuleStatus) DeepCopyInto(out *FirewallRuleStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FirewallRuleStatus. +func (in *FirewallRuleStatus) DeepCopy() *FirewallRuleStatus { + if in == nil { + return nil + } + out := new(FirewallRuleStatus) in.DeepCopyInto(out) return out } @@ -561,18 +635,40 @@ func (in *LinodeFirewallSpec) DeepCopyInto(out *LinodeFirewallSpec) { } if in.InboundRules != nil { in, out := &in.InboundRules, &out.InboundRules - *out = make([]FirewallRule, len(*in)) + *out = make([]FirewallRuleSpec, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.InboundRuleRefs != nil { + in, out := &in.InboundRuleRefs, &out.InboundRuleRefs + *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.OutboundRules != nil { in, out := &in.OutboundRules, &out.OutboundRules - *out = make([]FirewallRule, len(*in)) + *out = make([]FirewallRuleSpec, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.OutboundRuleRefs != nil { + in, out := &in.OutboundRuleRefs, &out.OutboundRuleRefs + *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.CredentialsRef != nil { in, out := &in.CredentialsRef, &out.CredentialsRef *out = new(v1.SecretReference) diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_firewallrules.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_firewallrules.yaml new file mode 100644 index 000000000..278d011eb --- /dev/null +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_firewallrules.yaml @@ -0,0 +1,143 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.16.5 + labels: + clusterctl.cluster.x-k8s.io/move-hierarchy: "true" + name: firewallrules.infrastructure.cluster.x-k8s.io +spec: + group: infrastructure.cluster.x-k8s.io + names: + categories: + - cluster-api + kind: FirewallRule + listKind: FirewallRuleList + plural: firewallrules + shortNames: + - fwr + singular: firewallrule + scope: Namespaced + versions: + - name: v1alpha2 + schema: + openAPIV3Schema: + description: FirewallRule is the Schema for the firewallrules API + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: FirewallRuleSpec defines the desired state of FirewallRule + properties: + action: + description: |- + INSERT ADDITIONAL SPEC FIELDS - desired state of cluster + Important: Run "make" to regenerate code after modifying this file + type: string + addressSetRefs: + description: |- + AddressSetRefs is a list of references to AddressSets as an alternative to + using Addresses but can be used in conjunction with it + items: + description: ObjectReference contains enough information to let + you inspect or modify the referred object. + 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. + 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 + addresses: + description: |- + NetworkAddresses holds a list of IPv4 and IPv6 addresses + We don't use linodego here since kubebuilder can't generate DeepCopyInto + for linodego.NetworkAddresses + properties: + ipv4: + items: + type: string + type: array + ipv6: + items: + type: string + type: array + type: object + description: + type: string + label: + type: string + ports: + type: string + protocol: + description: NetworkProtocol enum type + enum: + - TCP + - UDP + - ICMP + - IPENCAP + type: string + required: + - action + - label + - protocol + type: object + status: + description: FirewallRuleStatus defines the observed state of FirewallRule + type: object + type: object + served: true + storage: true + subresources: + status: {} 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 1caaf981a..09b070d2a 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodefirewalls.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodefirewalls.yaml @@ -78,10 +78,63 @@ spec: - ACCEPT - DROP type: string + inboundRuleRefs: + description: |- + InboundRuleRefs is a list of references to FirewallRules as an alternative to + using InboundRules but can be used in conjunction with it + items: + description: ObjectReference contains enough information to let + you inspect or modify the referred object. + 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. + 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 inboundRules: items: + description: FirewallRuleSpec defines the desired state of FirewallRule properties: action: + description: |- + INSERT ADDITIONAL SPEC FIELDS - desired state of cluster + Important: Run "make" to regenerate code after modifying this file type: string addressSetRefs: description: |- @@ -175,10 +228,63 @@ spec: - ACCEPT - DROP type: string + outboundRuleRefs: + description: |- + OutboundRuleRefs is a list of references to FirewallRules as an alternative to + using OutboundRules but can be used in conjunction with it + items: + description: ObjectReference contains enough information to let + you inspect or modify the referred object. + 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. + 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 outboundRules: items: + description: FirewallRuleSpec defines the desired state of FirewallRule properties: action: + description: |- + INSERT ADDITIONAL SPEC FIELDS - desired state of cluster + Important: Run "make" to regenerate code after modifying this file type: string addressSetRefs: description: |- diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 8a099af4f..908197eb4 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -19,6 +19,7 @@ resources: - bases/infrastructure.cluster.x-k8s.io_linodeobjectstoragekeys.yaml - bases/infrastructure.cluster.x-k8s.io_linodefirewalls.yaml - bases/infrastructure.cluster.x-k8s.io_addresssets.yaml +- bases/infrastructure.cluster.x-k8s.io_firewallrules.yaml #+kubebuilder:scaffold:crdkustomizeresource patches: @@ -40,6 +41,7 @@ patches: - path: patches/webhook_in_linodemachinetemplates.yaml #- path: patches/webhook_in_linodefirewalls.yaml #- path: patches/webhook_in_addresssets.yaml +#- path: patches/webhook_in_firewallrules.yaml #+kubebuilder:scaffold:crdkustomizewebhookpatch - path: patches/capicontract_in_linodeclusters.yaml @@ -63,6 +65,7 @@ patches: #- path: patches/cainjection_in_linodeobjectstoragebuckets.yaml #- path: patches/cainjection_in_linodefirewalls.yaml #- path: patches/cainjection_in_addresssets.yaml +#- path: patches/cainjection_in_firewallrules.yaml #+kubebuilder:scaffold:crdkustomizecainjectionpatch # [VALIDATION] diff --git a/config/crd/patches/cainjection_in_firewallrules.yaml b/config/crd/patches/cainjection_in_firewallrules.yaml new file mode 100644 index 000000000..4424aecc9 --- /dev/null +++ b/config/crd/patches/cainjection_in_firewallrules.yaml @@ -0,0 +1,7 @@ +# The following patch adds a directive for certmanager to inject CA into the CRD +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + cert-manager.io/inject-ca-from: CERTIFICATE_NAMESPACE/CERTIFICATE_NAME + name: firewallrules.infrastructure.cluster.x-k8s.io diff --git a/config/crd/patches/webhook_in_firewallrules.yaml b/config/crd/patches/webhook_in_firewallrules.yaml new file mode 100644 index 000000000..ae1ffd779 --- /dev/null +++ b/config/crd/patches/webhook_in_firewallrules.yaml @@ -0,0 +1,16 @@ +# The following patch enables a conversion webhook for the CRD +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: firewallrules.infrastructure.cluster.x-k8s.io +spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + namespace: system + name: webhook-service + path: /convert + conversionReviewVersions: + - v1 diff --git a/config/rbac/firewallrule_editor_role.yaml b/config/rbac/firewallrule_editor_role.yaml new file mode 100644 index 000000000..72ae35a7c --- /dev/null +++ b/config/rbac/firewallrule_editor_role.yaml @@ -0,0 +1,31 @@ +# permissions for end users to edit firewallrules. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + labels: + app.kubernetes.io/name: clusterrole + app.kubernetes.io/instance: firewallrule-editor-role + app.kubernetes.io/component: rbac + app.kubernetes.io/created-by: cluster-api-provider-linode + app.kubernetes.io/part-of: cluster-api-provider-linode + app.kubernetes.io/managed-by: kustomize + name: firewallrule-editor-role +rules: +- apiGroups: + - infrastructure.cluster.x-k8s.io + resources: + - firewallrules + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - infrastructure.cluster.x-k8s.io + resources: + - firewallrules/status + verbs: + - get diff --git a/config/rbac/firewallrule_viewer_role.yaml b/config/rbac/firewallrule_viewer_role.yaml new file mode 100644 index 000000000..0d8cb0b13 --- /dev/null +++ b/config/rbac/firewallrule_viewer_role.yaml @@ -0,0 +1,27 @@ +# permissions for end users to view firewallrules. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + labels: + app.kubernetes.io/name: clusterrole + app.kubernetes.io/instance: firewallrule-viewer-role + app.kubernetes.io/component: rbac + app.kubernetes.io/created-by: cluster-api-provider-linode + app.kubernetes.io/part-of: cluster-api-provider-linode + app.kubernetes.io/managed-by: kustomize + name: firewallrule-viewer-role +rules: +- apiGroups: + - infrastructure.cluster.x-k8s.io + resources: + - firewallrules + verbs: + - get + - list + - watch +- apiGroups: + - infrastructure.cluster.x-k8s.io + resources: + - firewallrules/status + verbs: + - get diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 6c46ea0e5..701772f1f 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -30,6 +30,7 @@ rules: - infrastructure.cluster.x-k8s.io resources: - addresssets + - firewallrules verbs: - get - list @@ -40,6 +41,7 @@ rules: - infrastructure.cluster.x-k8s.io resources: - addresssets/finalizers + - firewallrules/finalizers - linodeclusters/finalizers - linodefirewalls/finalizers - linodemachines/finalizers diff --git a/config/samples/infrastructure_v1alpha2_firewallrule.yaml b/config/samples/infrastructure_v1alpha2_firewallrule.yaml new file mode 100644 index 000000000..a8c0d0392 --- /dev/null +++ b/config/samples/infrastructure_v1alpha2_firewallrule.yaml @@ -0,0 +1,12 @@ +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 +kind: FirewallRule +metadata: + labels: + app.kubernetes.io/name: firewallrule + app.kubernetes.io/instance: firewallrule-sample + app.kubernetes.io/part-of: cluster-api-provider-linode + app.kubernetes.io/managed-by: kustomize + app.kubernetes.io/created-by: cluster-api-provider-linode + name: firewallrule-sample +spec: + # TODO(user): Add fields here diff --git a/config/samples/kustomization.yaml b/config/samples/kustomization.yaml index 8acc21f13..64a09e06e 100644 --- a/config/samples/kustomization.yaml +++ b/config/samples/kustomization.yaml @@ -8,4 +8,5 @@ resources: - infrastructure_v1alpha2_linodemachinetemplate.yaml - infrastructure_v1alpha2_linodefirewall.yaml - infrastructure_v1alpha2_addressset.yaml +- infrastructure_v1alpha2_firewallrule.yaml #+kubebuilder:scaffold:manifestskustomizesamples diff --git a/docs/src/topics/firewalling.md b/docs/src/topics/firewalling.md index b755620af..13108ff54 100644 --- a/docs/src/topics/firewalling.md +++ b/docs/src/topics/firewalling.md @@ -90,7 +90,7 @@ For controlling firewalls via Linode resources, a [Cloud Firewall](https://www.l be defined and provisioned via the `LinodeFirewall` resource in CAPL. Any updates to the cloud firewall CAPL resource will be updated in the cloud firewall and overwrite any changes made outside the CAPL resource. -Example `LinodeFirewall` and `AddressSet`: +Example `LinodeFirewall`, `AddressSet`, and `FirewallRule`: ```yaml apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 kind: LinodeFirewall @@ -100,30 +100,61 @@ spec: enabled: true inboundPolicy: DROP inboundRules: - - action: ACCEPT - label: intra-cluster - ports: "1-65535" - protocol: "TCP" - addresses: - ipv4: - - "10.0.0.0/8" - action: ACCEPT label: inbound-api-server ports: "6443" protocol: TCP + addresses: + ipv4: + - "192.168.255.0/24" + - action: ACCEPT + label: intra-cluster + ports: "1-65535" + protocol: "TCP" addressSetRefs: # Can be used together with .addresses if desired. - - name: my-hosts + - name: vpc-addrset kind: AddressSet + inboundRuleRefs: # Can be used together with .inboundRules if desired + - name: example-fwrule-udp + kind: FirewallRule + - name: example-fwrule-icmp + kind: FirewallRule + # outboundRules: [] + # outboundRuleRefs: [] + # outboundPolicy: ACCEPT --- apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 kind: AddressSet metadata: - name: my-hosts + name: vpc-addrset spec: ipv4: - - "0.0.0.0/0" - ipv6: - - ::/0 + - "10.0.0.0/8" +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 +kind: FirewallRule +metadata: + name: example-fwrule-udp +spec: + action: ACCEPT + label: intra-cluster-udp + ports: "1-65535" + protocol: "UDP" + addresses: + ipv4: + - "10.0.0.0/8" +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 +kind: FirewallRule +metadata: + name: example-fwrule-icmp +spec: + action: ACCEPT + label: intra-cluster-icmp + protocol: "ICMP" + addressSetRefs: # Can be used together with .addresses if desired. + - name: vpc-addrset + kind: AddressSet ``` ### Cloud Firewall Machine Integration diff --git a/e2e/linodefirewall-controller/linodefirewall-firewall-rule/assert-capi-resources.yaml b/e2e/linodefirewall-controller/linodefirewall-firewall-rule/assert-capi-resources.yaml new file mode 100644 index 000000000..8a5b8dbab --- /dev/null +++ b/e2e/linodefirewall-controller/linodefirewall-firewall-rule/assert-capi-resources.yaml @@ -0,0 +1,15 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: capi-controller-manager + namespace: capi-system +status: + availableReplicas: 1 +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: capl-controller-manager + namespace: capl-system +status: + availableReplicas: 1 diff --git a/e2e/linodefirewall-controller/linodefirewall-firewall-rule/assert-fwrule-firewall.yaml b/e2e/linodefirewall-controller/linodefirewall-firewall-rule/assert-fwrule-firewall.yaml new file mode 100644 index 000000000..a1b877a8c --- /dev/null +++ b/e2e/linodefirewall-controller/linodefirewall-firewall-rule/assert-fwrule-firewall.yaml @@ -0,0 +1,27 @@ +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 +kind: LinodeFirewall +metadata: + name: ($firewall) +spec: + enabled: true + inboundPolicy: DROP + inboundRuleRefs: + - name: ($fwrule) + kind: FirewallRule +status: + ready: true +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 +kind: FirewallRule +metadata: + name: ($fwrule) +spec: + action: ACCEPT + label: test + ports: "1-65535" + protocol: "TCP" + addresses: + ipv4: + - "10.0.0.0/8" + ipv6: + - ::/0 diff --git a/e2e/linodefirewall-controller/linodefirewall-firewall-rule/chainsaw-test.yaml b/e2e/linodefirewall-controller/linodefirewall-firewall-rule/chainsaw-test.yaml new file mode 100644 index 000000000..a01e95ea3 --- /dev/null +++ b/e2e/linodefirewall-controller/linodefirewall-firewall-rule/chainsaw-test.yaml @@ -0,0 +1,88 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: linodefirewall-firewallrule + # Label to trigger the test on every PR + labels: + all: + quick: + linodefirewall: +spec: + bindings: + # A short identifier for the E2E test run + - name: run + value: (join('-', ['e2e', 'fwrule-firewall', env('GIT_REF')])) + - name: firewall + # Format the firewall name into a valid Kubernetes object name + value: (trim((truncate(($run), `63`)), '-')) + - name: fwrule + # Format the fwrule name into a valid Kubernetes object name + value: (trim((truncate(($run), `63`)), '-')) + template: true + steps: + - name: Check if CAPI provider resources exist + try: + - assert: + file: assert-capi-resources.yaml + - name: Create FirewallRule and LinodeFirewall + try: + - apply: + file: create-fwrule-firewall.yaml + - assert: + file: assert-fwrule-firewall.yaml + catch: + - describe: + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 + kind: LinodeFirewall + - describe: + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 + kind: FirewallRule + - name: Check if the Firewall was created + try: + - script: + env: + - name: FILTER + value: (to_string({"label":($firewall)})) + content: | + set -e + curl -s \ + -H "Authorization: Bearer $LINODE_TOKEN" \ + -H "X-Filter: $FILTER" \ + -H "Content-Type: application/json" \ + "https://api.linode.com/v4/networking/firewalls" + check: + ($error): ~ + (json_parse($stdout)): + results: 1 + - name: Delete Firewall and FirewallRule + try: + - delete: + ref: + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 + kind: LinodeFirewall + name: ($firewall) + - delete: + ref: + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 + kind: FirewallRule + name: ($fwrule) + - error: + file: check-fwrule-firewall-deletion.yaml + - name: Check if the Firewall was deleted + try: + - script: + env: + - name: FILTER + value: (to_string({"label":($firewall)})) + content: | + set -e + curl -s \ + -H "Authorization: Bearer $LINODE_TOKEN" \ + -H "X-Filter: $FILTER" \ + -H "Content-Type: application/json" \ + "https://api.linode.com/v4/networking/firewalls" + check: + ($error): ~ + (json_parse($stdout)): + results: 0 diff --git a/e2e/linodefirewall-controller/linodefirewall-firewall-rule/check-fwrule-firewall-deletion.yaml b/e2e/linodefirewall-controller/linodefirewall-firewall-rule/check-fwrule-firewall-deletion.yaml new file mode 100644 index 000000000..6f44c3599 --- /dev/null +++ b/e2e/linodefirewall-controller/linodefirewall-firewall-rule/check-fwrule-firewall-deletion.yaml @@ -0,0 +1,25 @@ +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 +kind: LinodeFirewall +metadata: + name: ($firewall) +spec: + enabled: true + inboundPolicy: DROP + inboundRuleRefs: + - name: ($fwrule) + kind: FirewallRule +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 +kind: FirewallRule +metadata: + name: ($fwrule) +spec: + action: ACCEPT + label: test + ports: "1-65535" + protocol: "TCP" + addresses: + ipv4: + - "10.0.0.0/8" + ipv6: + - ::/0 diff --git a/e2e/linodefirewall-controller/linodefirewall-firewall-rule/create-fwrule-firewall.yaml b/e2e/linodefirewall-controller/linodefirewall-firewall-rule/create-fwrule-firewall.yaml new file mode 100644 index 000000000..6f44c3599 --- /dev/null +++ b/e2e/linodefirewall-controller/linodefirewall-firewall-rule/create-fwrule-firewall.yaml @@ -0,0 +1,25 @@ +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 +kind: LinodeFirewall +metadata: + name: ($firewall) +spec: + enabled: true + inboundPolicy: DROP + inboundRuleRefs: + - name: ($fwrule) + kind: FirewallRule +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 +kind: FirewallRule +metadata: + name: ($fwrule) +spec: + action: ACCEPT + label: test + ports: "1-65535" + protocol: "TCP" + addresses: + ipv4: + - "10.0.0.0/8" + ipv6: + - ::/0 diff --git a/go.mod b/go.mod index e92f72e75..7a1b0f8cc 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,6 @@ go 1.23 require ( github.com/akamai/AkamaiOPEN-edgegrid-golang/v8 v8.4.0 github.com/go-logr/logr v1.4.2 - github.com/google/go-cmp v0.6.0 github.com/google/uuid v1.6.0 github.com/linode/linodego v1.43.0 github.com/onsi/ginkgo/v2 v2.22.0 @@ -34,6 +33,7 @@ require ( github.com/felixge/httpsnoop v1.0.4 // indirect github.com/fxamacker/cbor/v2 v2.7.0 // indirect github.com/google/cel-go v0.20.1 // indirect + github.com/google/go-cmp v0.6.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/spf13/cobra v1.8.1 // indirect github.com/stoewer/go-strcase v1.2.0 // indirect diff --git a/internal/controller/linodefirewall_controller.go b/internal/controller/linodefirewall_controller.go index bbacb0bcc..992c367c7 100644 --- a/internal/controller/linodefirewall_controller.go +++ b/internal/controller/linodefirewall_controller.go @@ -63,6 +63,8 @@ type LinodeFirewallReconciler struct { //+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodefirewalls/finalizers,verbs=update //+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=addresssets,verbs=get;list;watch;update;patch //+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=addresssets/finalizers,verbs=update +//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=firewallrules,verbs=get;list;watch;update;patch +//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=firewallrules/finalizers,verbs=update func (r *LinodeFirewallReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultedLoopTimeout(r.ReconcileTimeout)) @@ -205,23 +207,47 @@ func (r *LinodeFirewallReconciler) reconcile( return ctrl.Result{}, nil } -func (r *LinodeFirewallReconciler) removeAddressSetFinalizer(ctx context.Context, logger logr.Logger, fwScope *scope.FirewallScope, addrSetRef *corev1.ObjectReference) error { - if addrSetRef == nil || r.Client == nil { +func (r *LinodeFirewallReconciler) removeAddressSetFinalizer(ctx context.Context, logger logr.Logger, fwScope *scope.FirewallScope, objRef *corev1.ObjectReference) error { + if objRef == nil || r.Client == nil { return nil } addrSet := &infrav1alpha2.AddressSet{} - if addrSetRef.Namespace == "" { - addrSetRef.Namespace = fwScope.LinodeFirewall.Namespace + if objRef.Namespace == "" { + objRef.Namespace = fwScope.LinodeFirewall.Namespace } - if err := r.Client.Get(ctx, client.ObjectKey{Namespace: addrSetRef.Namespace, Name: addrSetRef.Name}, addrSet); err != nil { + if err := r.Client.Get(ctx, client.ObjectKey{Namespace: objRef.Namespace, Name: objRef.Name}, addrSet); err != nil { if !apierrors.IsNotFound(err) { - logger.Error(err, "failed to fetch referenced AddressSet", "namespace", addrSetRef.Namespace, "name", addrSetRef.Name) + logger.Error(err, "failed to fetch referenced Object", "kind", objRef.Kind, "namespace", objRef.Namespace, "name", objRef.Name) return err } } if controllerutil.RemoveFinalizer(addrSet, getFinalizer(fwScope.LinodeFirewall)) { if err := r.Update(ctx, addrSet); err != nil { - logger.Error(err, "failed to remove finalizer from AddressSet") + logger.Error(err, "failed to remove finalizer from Object", "kind", objRef.Kind, "namespace", objRef.Namespace, "name", objRef.Name) + return err + } + } + + return nil +} + +func (r *LinodeFirewallReconciler) removeFirewallRuleFinalizer(ctx context.Context, logger logr.Logger, fwScope *scope.FirewallScope, objRef *corev1.ObjectReference) error { + if objRef == nil || r.Client == nil { + return nil + } + firewallRule := &infrav1alpha2.FirewallRule{} + if objRef.Namespace == "" { + objRef.Namespace = fwScope.LinodeFirewall.Namespace + } + if err := r.Client.Get(ctx, client.ObjectKey{Namespace: objRef.Namespace, Name: objRef.Name}, firewallRule); err != nil { + if !apierrors.IsNotFound(err) { + logger.Error(err, "failed to fetch referenced Object", "kind", objRef.Kind, "namespace", objRef.Namespace, "name", objRef.Name) + return err + } + } + if controllerutil.RemoveFinalizer(firewallRule, getFinalizer(fwScope.LinodeFirewall)) { + if err := r.Update(ctx, firewallRule); err != nil { + logger.Error(err, "failed to remove finalizer from Object", "kind", objRef.Kind, "namespace", objRef.Namespace, "name", objRef.Name) return err } } @@ -262,6 +288,17 @@ func (r *LinodeFirewallReconciler) reconcileDelete( } } } + // remove finalizers on any FirewallRules referenced in the firewall + for _, inboundRuleRef := range fwScope.LinodeFirewall.Spec.InboundRuleRefs { + if err := r.removeFirewallRuleFinalizer(ctx, logger, fwScope, inboundRuleRef); err != nil { + return ctrl.Result{}, err + } + } + for _, outboundRuleRef := range fwScope.LinodeFirewall.Spec.OutboundRuleRefs { + if err := r.removeFirewallRuleFinalizer(ctx, logger, fwScope, outboundRuleRef); err != nil { + return ctrl.Result{}, err + } + } err := fwScope.LinodeClient.DeleteFirewall(ctx, *fwScope.LinodeFirewall.Spec.FirewallID) if util.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil { @@ -315,7 +352,12 @@ func (r *LinodeFirewallReconciler) SetupWithManager(mgr ctrl.Manager, options cr ). Watches( &infrav1alpha2.AddressSet{}, - handler.EnqueueRequestsFromMapFunc(findObjectsForAddressSet(mgr.GetLogger(), r.TracedClient())), + handler.EnqueueRequestsFromMapFunc(findObjectsForObject(mgr.GetLogger(), r.TracedClient())), + builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}), + ). + Watches( + &infrav1alpha2.FirewallRule{}, + handler.EnqueueRequestsFromMapFunc(findObjectsForObject(mgr.GetLogger(), r.TracedClient())), builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}), ). Complete(wrappedruntimereconciler.NewRuntimeReconcilerWithTracing(r, wrappedruntimereconciler.DefaultDecorator())) diff --git a/internal/controller/linodefirewall_controller_helpers.go b/internal/controller/linodefirewall_controller_helpers.go index fee9614b1..6a8a925c4 100644 --- a/internal/controller/linodefirewall_controller_helpers.go +++ b/internal/controller/linodefirewall_controller_helpers.go @@ -33,12 +33,17 @@ const ( maxRulesPerFirewall = 25 ) +const ( + ruleTypeInbound = "inbound" + ruleTypeOutbound = "outbound" +) + var ( errTooManyIPs = errors.New("too many IPs in this ACL, will exceed rules per firewall limit") ) -func findObjectsForAddressSet(logger logr.Logger, tracedClient client.Client) handler.MapFunc { - logger = logger.WithName("LinodeFirewallReconciler").WithName("findObjectsForAddressSet") +func findObjectsForObject(logger logr.Logger, tracedClient client.Client) handler.MapFunc { + logger = logger.WithName("LinodeFirewallReconciler").WithName("findObjectsForObject") return func(ctx context.Context, obj client.Object) []ctrl.Request { ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultMappingTimeout) defer cancel() @@ -63,7 +68,7 @@ func findObjectsForAddressSet(logger logr.Logger, tracedClient client.Client) ha } // Constructs a unique list of requests for updating LinodeFirewalls that either reference the -// AddressSet in the Inbound and/or OutboundRules +// AddressSet / FirewallRule func buildRequests(firewalls []infrav1alpha2.LinodeFirewall, obj client.Object) []reconcile.Request { requestSet := make(map[reconcile.Request]struct{}) for _, firewall := range firewalls { @@ -73,14 +78,16 @@ func buildRequests(firewalls []infrav1alpha2.LinodeFirewall, obj client.Object) for _, outboundRule := range firewall.Spec.OutboundRules { requestSet = buildRequestsHelper(requestSet, firewall, outboundRule.AddressSetRefs, obj) } + requestSet = buildRequestsHelper(requestSet, firewall, firewall.Spec.InboundRuleRefs, obj) + requestSet = buildRequestsHelper(requestSet, firewall, firewall.Spec.OutboundRuleRefs, obj) } return slices.Collect(maps.Keys(requestSet)) } -func buildRequestsHelper(requestSet map[reconcile.Request]struct{}, firewall infrav1alpha2.LinodeFirewall, addrSetRefs []*corev1.ObjectReference, obj client.Object) map[reconcile.Request]struct{} { - for _, addrSetRef := range addrSetRefs { - if addrSetRef.Name == obj.GetName() && addrSetRef.Namespace == obj.GetNamespace() { +func buildRequestsHelper(requestSet map[reconcile.Request]struct{}, firewall infrav1alpha2.LinodeFirewall, objRefs []*corev1.ObjectReference, obj client.Object) map[reconcile.Request]struct{} { + for _, objRef := range objRefs { + if objRef.Name == obj.GetName() && objRef.Namespace == obj.GetNamespace() { requestSet[reconcile.Request{ NamespacedName: types.NamespacedName{ Name: firewall.GetName(), @@ -232,52 +239,47 @@ func transformToCIDR(ip string) string { return ip } -// processInboundRule handles a single inbound rule -func processInboundRule(ctx context.Context, k8sClient clients.K8sClient, log logr.Logger, rule infrav1alpha2.FirewallRule, lfw *infrav1alpha2.LinodeFirewall, createOpts *linodego.FirewallCreateOptions) error { - var ruleIPv4s []string - var ruleIPv6s []string - var err error - if rule.Addresses != nil { - ruleIPv4s, ruleIPv6s = processAddresses(rule.Addresses) +func filterDuplicates(ipv4s, ipv6s []string) (filteredIPv4s, filteredIPv6s []string) { + // Declare "sets". Empty structs occupy 0 memory + ipv4Set := make(map[string]struct{}) + ipv6Set := make(map[string]struct{}) + for _, ip := range ipv4s { + ipv4Set[ip] = struct{}{} } - if rule.AddressSetRefs != nil { - ruleIPv4s, ruleIPv6s, err = processAddressSetRefs(ctx, k8sClient, lfw, rule.AddressSetRefs, log) - if err != nil { - return err - } + for _, ip := range ipv6s { + ipv6Set[ip] = struct{}{} } - ruleLabel := formatRuleLabel(rule.Action, rule.Label) - - // Process IPv4 - processIPv4Rules(ruleIPv4s, rule, ruleLabel, &createOpts.Rules.Inbound) - - // Process IPv6 - processIPv6Rules(ruleIPv6s, rule, ruleLabel, &createOpts.Rules.Inbound) - - return nil + return slices.Collect(maps.Keys(ipv4Set)), slices.Collect(maps.Keys(ipv6Set)) } -// processOutboundRule handles a single outbound rule -func processOutboundRule(ctx context.Context, k8sClient clients.K8sClient, log logr.Logger, rule infrav1alpha2.FirewallRule, lfw *infrav1alpha2.LinodeFirewall, createOpts *linodego.FirewallCreateOptions) error { - var ruleIPv4s []string - var ruleIPv6s []string - var err error +// processRule handles a single inbound/outbound rule +func processRule(ctx context.Context, k8sClient clients.K8sClient, log logr.Logger, rule infrav1alpha2.FirewallRuleSpec, lfw *infrav1alpha2.LinodeFirewall, ruleType string, createOpts *linodego.FirewallCreateOptions) error { + ruleIPv4s := make([]string, 0) + ruleIPv6s := make([]string, 0) if rule.Addresses != nil { - ruleIPv4s, ruleIPv6s = processAddresses(rule.Addresses) + ipv4s, ipv6s := processAddresses(rule.Addresses) + ruleIPv4s = append(ruleIPv4s, ipv4s...) + ruleIPv6s = append(ruleIPv6s, ipv6s...) } if rule.AddressSetRefs != nil { - ruleIPv4s, ruleIPv6s, err = processAddressSetRefs(ctx, k8sClient, lfw, rule.AddressSetRefs, log) + ipv4s, ipv6s, err := processAddressSetRefs(ctx, k8sClient, lfw, rule.AddressSetRefs, log) if err != nil { return err } + ruleIPv4s = append(ruleIPv4s, ipv4s...) + ruleIPv6s = append(ruleIPv6s, ipv6s...) } - ruleLabel := formatRuleLabel(lfw.Spec.OutboundPolicy, rule.Label) + ruleIPv4s, ruleIPv6s = filterDuplicates(ruleIPv4s, ruleIPv6s) - // Process IPv4 - processIPv4Rules(ruleIPv4s, rule, ruleLabel, &createOpts.Rules.Outbound) + ruleLabel := formatRuleLabel(rule.Action, rule.Label) - // Process IPv6 - processIPv6Rules(ruleIPv6s, rule, ruleLabel, &createOpts.Rules.Outbound) + if ruleType == "inbound" { + processIPRules(ruleIPv4s, rule, ruleLabel, linodego.IPTypeIPv4, &createOpts.Rules.Inbound) + processIPRules(ruleIPv6s, rule, ruleLabel, linodego.IPTypeIPv6, &createOpts.Rules.Inbound) + } else if ruleType == "outbound" { + processIPRules(ruleIPv4s, rule, ruleLabel, linodego.IPTypeIPv4, &createOpts.Rules.Outbound) + processIPRules(ruleIPv6s, rule, ruleLabel, linodego.IPTypeIPv6, &createOpts.Rules.Outbound) + } return nil } @@ -370,8 +372,8 @@ func formatRuleLabel(prefix, label string) string { return ruleLabel } -// processIPv4Rules processes IPv4 rules and adds them to the rules slice -func processIPv4Rules(ips []string, rule infrav1alpha2.FirewallRule, ruleLabel string, rules *[]linodego.FirewallRule) { +// processIPRules processes IP rules and adds them to the rules slice +func processIPRules(ips []string, rule infrav1alpha2.FirewallRuleSpec, ruleLabel string, ipType linodego.InstanceIPType, rules *[]linodego.FirewallRule) { // Initialize rules if nil if *rules == nil { *rules = make([]linodego.FirewallRule, 0) @@ -382,44 +384,56 @@ func processIPv4Rules(ips []string, rule infrav1alpha2.FirewallRule, ruleLabel s return } - ipv4chunks := chunkIPs(ips) - for i, chunk := range ipv4chunks { - v4chunk := chunk - *rules = append(*rules, linodego.FirewallRule{ - Action: rule.Action, - Label: ruleLabel, - Description: fmt.Sprintf("Rule %d, Created by CAPL: %s", i, rule.Label), - Protocol: rule.Protocol, - Ports: rule.Ports, - Addresses: linodego.NetworkAddresses{IPv4: &v4chunk}, - }) + ipchunks := chunkIPs(ips) + if ipType == linodego.IPTypeIPv4 { + for i, chunk := range ipchunks { + *rules = append(*rules, linodego.FirewallRule{ + Action: rule.Action, + Label: ruleLabel, + Description: fmt.Sprintf("Rule %d, Created by CAPL: %s", i, rule.Label), + Protocol: rule.Protocol, + Ports: rule.Ports, + Addresses: linodego.NetworkAddresses{IPv4: &chunk}, + }) + } + } else if ipType == linodego.IPTypeIPv6 { + for i, chunk := range ipchunks { + *rules = append(*rules, linodego.FirewallRule{ + Action: rule.Action, + Label: ruleLabel, + Description: fmt.Sprintf("Rule %d, Created by CAPL: %s", i, rule.Label), + Protocol: rule.Protocol, + Ports: rule.Ports, + Addresses: linodego.NetworkAddresses{IPv6: &chunk}, + }) + } } } -// processIPv6Rules processes IPv6 rules and adds them to the rules slice -func processIPv6Rules(ips []string, rule infrav1alpha2.FirewallRule, ruleLabel string, rules *[]linodego.FirewallRule) { - // Initialize rules if nil - if *rules == nil { - *rules = make([]linodego.FirewallRule, 0) +func processFirewallRule(ctx context.Context, k8sClient clients.K8sClient, log logr.Logger, ruleRef *corev1.ObjectReference, firewall *infrav1alpha2.LinodeFirewall, ruleType string, createOpts *linodego.FirewallCreateOptions) error { + rule := &infrav1alpha2.FirewallRule{} + if ruleRef.Namespace == "" { + ruleRef.Namespace = firewall.Namespace } + if k8sClient != nil { + if err := k8sClient.Get(ctx, client.ObjectKey{Namespace: ruleRef.Namespace, Name: ruleRef.Name}, rule); err != nil { + log.Error(err, "failed to fetch referenced AddressSet", "namespace", ruleRef.Namespace, "name", ruleRef.Name) - // If no IPs, return early - if len(ips) == 0 { - return + return err + } + finalizer := getFinalizer(firewall) + if !controllerutil.ContainsFinalizer(rule, finalizer) { + controllerutil.AddFinalizer(rule, finalizer) + if err := k8sClient.Update(ctx, rule); err != nil { + return err + } + } } - - ipv6chunks := chunkIPs(ips) - for i, chunk := range ipv6chunks { - v6chunk := chunk - *rules = append(*rules, linodego.FirewallRule{ - Action: rule.Action, - Label: ruleLabel, - Description: fmt.Sprintf("Rule %d, Created by CAPL: %s", i, rule.Label), - Protocol: rule.Protocol, - Ports: rule.Ports, - Addresses: linodego.NetworkAddresses{IPv6: &v6chunk}, - }) + if err := processRule(ctx, k8sClient, log, rule.Spec, firewall, ruleType, createOpts); err != nil { + return err } + + return nil } // processACL uses the CAPL LinodeFirewall representation to build out the inbound @@ -431,7 +445,12 @@ func processACL(ctx context.Context, k8sClient clients.K8sClient, log logr.Logge // Process inbound rules for _, rule := range firewall.Spec.InboundRules { - if err := processInboundRule(ctx, k8sClient, log, rule, firewall, createOpts); err != nil { + if err := processRule(ctx, k8sClient, log, rule, firewall, ruleTypeInbound, createOpts); err != nil { + return nil, err + } + } + for _, ruleRef := range firewall.Spec.InboundRuleRefs { + if err := processFirewallRule(ctx, k8sClient, log, ruleRef, firewall, ruleTypeInbound, createOpts); err != nil { return nil, err } } @@ -445,7 +464,12 @@ func processACL(ctx context.Context, k8sClient clients.K8sClient, log logr.Logge // Process outbound rules for _, rule := range firewall.Spec.OutboundRules { - if err := processOutboundRule(ctx, k8sClient, log, rule, firewall, createOpts); err != nil { + if err := processRule(ctx, k8sClient, log, rule, firewall, ruleTypeOutbound, createOpts); err != nil { + return nil, err + } + } + for _, ruleRef := range firewall.Spec.OutboundRuleRefs { + if err := processFirewallRule(ctx, k8sClient, log, ruleRef, firewall, ruleTypeOutbound, createOpts); err != nil { return nil, err } } diff --git a/internal/controller/linodefirewall_controller_helpers_test.go b/internal/controller/linodefirewall_controller_helpers_test.go index e5d258649..9c556f961 100644 --- a/internal/controller/linodefirewall_controller_helpers_test.go +++ b/internal/controller/linodefirewall_controller_helpers_test.go @@ -76,7 +76,7 @@ func TestProcessACL(t *testing.T) { name: "Single IP addresses are converted to CIDR", firewall: &infrav1alpha2.LinodeFirewall{ Spec: infrav1alpha2.LinodeFirewallSpec{ - InboundRules: []infrav1alpha2.FirewallRule{ + InboundRules: []infrav1alpha2.FirewallRuleSpec{ { Action: "ACCEPT", Label: "test-rule", @@ -123,7 +123,7 @@ func TestProcessACL(t *testing.T) { name: "Mixed single IPs and CIDR notation", firewall: &infrav1alpha2.LinodeFirewall{ Spec: infrav1alpha2.LinodeFirewallSpec{ - InboundRules: []infrav1alpha2.FirewallRule{ + InboundRules: []infrav1alpha2.FirewallRuleSpec{ { Action: "ACCEPT", Label: "test-rule", @@ -324,14 +324,14 @@ func TestProcessIPv4Rules(t *testing.T) { tests := []struct { name string ips []string - rule infrav1alpha2.FirewallRule + rule infrav1alpha2.FirewallRuleSpec ruleLabel string want []linodego.FirewallRule }{ { name: "Single IPv4 address", ips: []string{"192.168.1.1/32"}, - rule: infrav1alpha2.FirewallRule{ + rule: infrav1alpha2.FirewallRuleSpec{ Action: "ACCEPT", Protocol: "TCP", Ports: "80", @@ -352,7 +352,7 @@ func TestProcessIPv4Rules(t *testing.T) { { name: "Multiple IPv4 addresses within limit", ips: []string{"192.168.1.1/32", "10.0.0.0/8"}, - rule: infrav1alpha2.FirewallRule{ + rule: infrav1alpha2.FirewallRuleSpec{ Action: "DROP", Protocol: "UDP", Ports: "53", @@ -373,7 +373,7 @@ func TestProcessIPv4Rules(t *testing.T) { { name: "Empty IP list", ips: []string{}, - rule: infrav1alpha2.FirewallRule{ + rule: infrav1alpha2.FirewallRuleSpec{ Action: "ACCEPT", Protocol: "TCP", Ports: "80", @@ -389,7 +389,7 @@ func TestProcessIPv4Rules(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() var got []linodego.FirewallRule - processIPv4Rules(tt.ips, tt.rule, tt.ruleLabel, &got) + processIPRules(tt.ips, tt.rule, tt.ruleLabel, linodego.IPTypeIPv4, &got) if !reflect.DeepEqual(got, tt.want) { t.Errorf("processIPv4Rules() = %v, want %v", got, tt.want) } @@ -403,14 +403,14 @@ func TestProcessIPv6Rules(t *testing.T) { tests := []struct { name string ips []string - rule infrav1alpha2.FirewallRule + rule infrav1alpha2.FirewallRuleSpec ruleLabel string want []linodego.FirewallRule }{ { name: "Single IPv6 address", ips: []string{"2001:db8::1/128"}, - rule: infrav1alpha2.FirewallRule{ + rule: infrav1alpha2.FirewallRuleSpec{ Action: "ACCEPT", Protocol: "TCP", Ports: "80", @@ -431,7 +431,7 @@ func TestProcessIPv6Rules(t *testing.T) { { name: "Multiple IPv6 addresses within limit", ips: []string{"2001:db8::1/128", "2001:db8::/32"}, - rule: infrav1alpha2.FirewallRule{ + rule: infrav1alpha2.FirewallRuleSpec{ Action: "DROP", Protocol: "UDP", Ports: "53", @@ -452,7 +452,7 @@ func TestProcessIPv6Rules(t *testing.T) { { name: "Empty IP list", ips: []string{}, - rule: infrav1alpha2.FirewallRule{ + rule: infrav1alpha2.FirewallRuleSpec{ Action: "ACCEPT", Protocol: "TCP", Ports: "80", @@ -468,7 +468,7 @@ func TestProcessIPv6Rules(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() var got []linodego.FirewallRule - processIPv6Rules(tt.ips, tt.rule, tt.ruleLabel, &got) + processIPRules(tt.ips, tt.rule, tt.ruleLabel, linodego.IPTypeIPv6, &got) if !reflect.DeepEqual(got, tt.want) { t.Errorf("processIPv6Rules() = %v, want %v", got, tt.want) } @@ -489,7 +489,7 @@ func TestProcessInboundRule(t *testing.T) { name: "Process inbound rule with IPv4 and IPv6", firewall: &infrav1alpha2.LinodeFirewall{ Spec: infrav1alpha2.LinodeFirewallSpec{ - InboundRules: []infrav1alpha2.FirewallRule{{ + InboundRules: []infrav1alpha2.FirewallRuleSpec{{ Action: "ACCEPT", Label: "test-rule", Protocol: "TCP", @@ -499,6 +499,7 @@ func TestProcessInboundRule(t *testing.T) { IPv6: &[]string{"2001:db8::1"}, }, }}, + InboundPolicy: "DROP", }, }, createOpts: &linodego.FirewallCreateOptions{ @@ -535,10 +536,10 @@ func TestProcessInboundRule(t *testing.T) { t.Parallel() logger := logr.Logger{} for _, rule := range tt.firewall.Spec.InboundRules { - err := processInboundRule(context.Background(), k8sClient, logger, rule, tt.firewall, tt.createOpts) + err := processRule(context.Background(), k8sClient, logger, rule, tt.firewall, ruleTypeInbound, tt.createOpts) require.NoError(t, err) if !reflect.DeepEqual(tt.createOpts, tt.want) { - t.Errorf("processInboundRule() = %v, want %v", tt.createOpts, tt.want) + t.Errorf("processRule (inbound) \n got: %+v\n want %+v", tt.createOpts, tt.want) } } }) @@ -558,7 +559,7 @@ func TestProcessOutboundRule(t *testing.T) { name: "Process outbound rule with IPv4 and IPv6", firewall: &infrav1alpha2.LinodeFirewall{ Spec: infrav1alpha2.LinodeFirewallSpec{ - OutboundRules: []infrav1alpha2.FirewallRule{{ + OutboundRules: []infrav1alpha2.FirewallRuleSpec{{ Action: "ACCEPT", Label: "test-rule", Protocol: "TCP", @@ -579,7 +580,7 @@ func TestProcessOutboundRule(t *testing.T) { Outbound: []linodego.FirewallRule{ { Action: "ACCEPT", - Label: "DROP-test-rule", + Label: "ACCEPT-test-rule", Description: "Rule 0, Created by CAPL: test-rule", Protocol: "TCP", Ports: "80", @@ -587,7 +588,7 @@ func TestProcessOutboundRule(t *testing.T) { }, { Action: "ACCEPT", - Label: "DROP-test-rule", + Label: "ACCEPT-test-rule", Description: "Rule 0, Created by CAPL: test-rule", Protocol: "TCP", Ports: "80", @@ -605,10 +606,10 @@ func TestProcessOutboundRule(t *testing.T) { t.Parallel() logger := logr.Logger{} for _, rule := range tt.firewall.Spec.OutboundRules { - err := processOutboundRule(context.Background(), k8sClient, logger, rule, tt.firewall, tt.createOpts) + err := processRule(context.Background(), k8sClient, logger, rule, tt.firewall, ruleTypeOutbound, tt.createOpts) require.NoError(t, err) if !reflect.DeepEqual(tt.createOpts, tt.want) { - t.Errorf("processOutboundRule() = %v, want %v", tt.createOpts, tt.want) + t.Errorf("processRule (outbound) \n got: %+v\n want %+v", tt.createOpts, tt.want) } } }) diff --git a/internal/controller/linodefirewall_controller_test.go b/internal/controller/linodefirewall_controller_test.go index f02feb684..7c71216cb 100644 --- a/internal/controller/linodefirewall_controller_test.go +++ b/internal/controller/linodefirewall_controller_test.go @@ -46,7 +46,8 @@ var _ = Describe("lifecycle", Ordered, Label("firewalls", "lifecycle"), func() { suite := NewControllerSuite(GinkgoT(), mock.MockLinodeClient{}) addrSetRefs := []*corev1.ObjectReference{{Namespace: defaultNamespace, Name: "lifecycle"}} - inboundRules := []infrav1alpha2.FirewallRule{{ + inboundRuleRefs := []*corev1.ObjectReference{{Namespace: defaultNamespace, Name: "lifecycle"}} + inboundRules := []infrav1alpha2.FirewallRuleSpec{{ Action: "ACCEPT", Label: "a-label-that-is-way-too-long-and-should-be-truncated", Description: "allow-ssh", @@ -58,7 +59,7 @@ var _ = Describe("lifecycle", Ordered, Label("firewalls", "lifecycle"), func() { }, AddressSetRefs: addrSetRefs, }} - outboundRules := []infrav1alpha2.FirewallRule{{ + outboundRules := []infrav1alpha2.FirewallRuleSpec{{ Action: "DROP", Label: "another-label-that-is-way-too-long-and-should-be-truncated", Description: "deny-foo", @@ -75,12 +76,13 @@ var _ = Describe("lifecycle", Ordered, Label("firewalls", "lifecycle"), func() { Namespace: defaultNamespace, }, Spec: infrav1alpha2.LinodeFirewallSpec{ - FirewallID: nil, - Enabled: true, - InboundRules: inboundRules, - OutboundRules: outboundRules, - InboundPolicy: "DROP", - OutboundPolicy: "ACCEPT", + FirewallID: nil, + Enabled: true, + InboundRules: inboundRules, + InboundRuleRefs: inboundRuleRefs, + OutboundRules: outboundRules, + InboundPolicy: "DROP", + OutboundPolicy: "ACCEPT", }, } addrSet := infrav1alpha2.AddressSet{ @@ -93,9 +95,26 @@ var _ = Describe("lifecycle", Ordered, Label("firewalls", "lifecycle"), func() { IPv6: &[]string{"::/0"}, }, } + fwRule := infrav1alpha2.FirewallRule{ + ObjectMeta: metav1.ObjectMeta{ + Name: "lifecycle", + Namespace: defaultNamespace, + }, + Spec: infrav1alpha2.FirewallRuleSpec{ + Action: "ACCEPT", + Label: "fwrule-test", + Description: "allow-ssh", + Ports: "22", + Protocol: "TCP", + Addresses: &infrav1alpha2.NetworkAddresses{ + IPv4: &[]string{"192.168.0.0/16"}, + }, + }, + } fwObjectKey := client.ObjectKeyFromObject(&linodeFW) addrSetObjectKey := client.ObjectKeyFromObject(&addrSet) + fwRuleObjectKey := client.ObjectKeyFromObject(&fwRule) var reconciler LinodeFirewallReconciler var fwScope scope.FirewallScope @@ -103,6 +122,7 @@ var _ = Describe("lifecycle", Ordered, Label("firewalls", "lifecycle"), func() { BeforeAll(func(ctx SpecContext) { fwScope.Client = k8sClient Expect(k8sClient.Create(ctx, &addrSet)).To(Succeed()) + Expect(k8sClient.Create(ctx, &fwRule)).To(Succeed()) Expect(k8sClient.Create(ctx, &linodeFW)).To(Succeed()) }) @@ -112,6 +132,7 @@ var _ = Describe("lifecycle", Ordered, Label("firewalls", "lifecycle"), func() { Expect(k8sClient.Get(ctx, fwObjectKey, &linodeFW)).To(Succeed()) fwScope.LinodeFirewall = &linodeFW Expect(k8sClient.Get(ctx, addrSetObjectKey, &addrSet)).To(Succeed()) + Expect(k8sClient.Get(ctx, fwRuleObjectKey, &fwRule)).To(Succeed()) // Create patch helper with latest state of resource. // This is only needed when relying on envtest's k8sClient. @@ -148,7 +169,7 @@ var _ = Describe("lifecycle", Ordered, Label("firewalls", "lifecycle"), func() { ), Path(Result("unable to create with too many rules", func(ctx context.Context, mck Mock) { for idx := 0; idx < 255; idx++ { - linodeFW.Spec.InboundRules = append(linodeFW.Spec.InboundRules, infrav1alpha2.FirewallRule{ + linodeFW.Spec.InboundRules = append(linodeFW.Spec.InboundRules, infrav1alpha2.FirewallRuleSpec{ Action: "ACCEPT", Ports: "22", Protocol: "TCP", @@ -219,7 +240,7 @@ var _ = Describe("lifecycle", Ordered, Label("firewalls", "lifecycle"), func() { for idx := 0; idx < 256; idx++ { ipv4s = append(ipv4s, fmt.Sprintf("192.168.%d.%d", idx, 0)) } - linodeFW.Spec.InboundRules = append(linodeFW.Spec.InboundRules, infrav1alpha2.FirewallRule{ + linodeFW.Spec.InboundRules = append(linodeFW.Spec.InboundRules, infrav1alpha2.FirewallRuleSpec{ Action: "ACCEPT", Ports: "22", Protocol: "TCP",