diff --git a/api/v1/clusterwidenetworkpolicy_types.go b/api/v1/clusterwidenetworkpolicy_types.go index e8b9d3fa..740df40f 100644 --- a/api/v1/clusterwidenetworkpolicy_types.go +++ b/api/v1/clusterwidenetworkpolicy_types.go @@ -10,7 +10,6 @@ import ( corev1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" ) type IPVersion string @@ -100,7 +99,7 @@ type IngressRule struct { // If this field is present and contains at least one item, then this rule allows // traffic only if the traffic matches at least one port in the list. // +optional - Ports []networking.NetworkPolicyPort `json:"ports,omitempty"` + Ports []NetworkPolicyPort `json:"ports,omitempty"` // List of sources which should be able to access the cluster for this rule. // Items in this list are combined using a logical OR operation. If this field is @@ -120,7 +119,7 @@ type EgressRule struct { // If this field is present and contains at least one item, then this rule allows // traffic only if the traffic matches at least one port in the list. // +optional - Ports []networking.NetworkPolicyPort `json:"ports,omitempty"` + Ports []NetworkPolicyPort `json:"ports,omitempty"` // List of destinations for outgoing traffic of a cluster for this rule. // Items in this list are combined using a logical OR operation. If this field is @@ -139,6 +138,24 @@ type EgressRule struct { ToFQDNs []FQDNSelector `json:"toFQDNs,omitempty"` } +// NetworkPolicyPort describes a port to allow traffic on +type NetworkPolicyPort struct { + // protocol represents the protocol (TCP, UDP) which traffic must match. + // If not specified, this field defaults to TCP. + // +optional + Protocol *corev1.Protocol `json:"protocol,omitempty"` + + // port represents the port on the given protocol. + Port int32 `json:"port,omitempty"` + + // endPort indicates that the range of ports from port to endPort if set, inclusive, + // should be allowed by the policy. This field cannot be defined if the port field + // is not defined. + // The endPort must be equal or greater than port. + // +optional + EndPort *int32 `json:"endPort,omitempty"` +} + // FQDNSelector describes rules for matching DNS names. type FQDNSelector struct { // MatchName matches FQDN. @@ -223,14 +240,10 @@ func (p *PolicySpec) Validate() error { return errors.Join(errs...) } -func validatePorts(ports []networking.NetworkPolicyPort) error { +func validatePorts(ports []NetworkPolicyPort) error { var errs []error for _, p := range ports { - if p.Port != nil && p.Port.Type != intstr.Int { - errs = append(errs, fmt.Errorf("only int ports are supported, but %v given", p.Port)) - } - - if p.Port != nil && (p.Port.IntValue() > 65535 || p.Port.IntValue() <= 0) { + if p.Port > 65535 || p.Port <= 0 { errs = append(errs, fmt.Errorf("only ports between 0 and 65535 are allowed, but %v given", p.Port)) } diff --git a/api/v1/clusterwidenetworkpolicy_types_test.go b/api/v1/clusterwidenetworkpolicy_types_test.go index 019b5599..909b15fc 100644 --- a/api/v1/clusterwidenetworkpolicy_types_test.go +++ b/api/v1/clusterwidenetworkpolicy_types_test.go @@ -5,16 +5,17 @@ import ( corev1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1" - "k8s.io/apimachinery/pkg/util/intstr" ) func TestPolicySpec_Validate(t *testing.T) { - tcp := corev1.ProtocolTCP - udp := corev1.ProtocolUDP - port1 := intstr.FromInt(8080) - port2 := intstr.FromInt(8081) - invalid := intstr.FromString("invalid") - invalidPort := intstr.FromInt(99999) + var ( + tcp = corev1.ProtocolTCP + udp = corev1.ProtocolUDP + port1 = int32(8080) + port2 = int32(8081) + invalidPort = int32(99999) + ) + tests := []struct { name string Ingress []IngressRule @@ -35,46 +36,22 @@ func TestPolicySpec_Validate(t *testing.T) { Except: []string{"192.168.0.1/32"}, }, }, - Ports: []networking.NetworkPolicyPort{ + Ports: []NetworkPolicyPort{ { Protocol: nil, - Port: &port1, + Port: port1, }, { Protocol: &tcp, - Port: &port2, + Port: port2, }, { Protocol: &udp, - Port: &port2, - }, - }, - }, - }, - }, - { - name: "invalid test", - Ingress: []IngressRule{ - { - From: []networking.IPBlock{ - { - CIDR: "1.1.0.0/24", - Except: []string{"1.1.1.0/16"}, - }, - { - CIDR: "192.168.0.1", - Except: []string{"192.168.0.2"}, - }, - }, - Ports: []networking.NetworkPolicyPort{ - { - Protocol: nil, - Port: &invalid, + Port: port2, }, }, }, }, - wantErr: true, }, { name: "invalid port", @@ -85,10 +62,10 @@ func TestPolicySpec_Validate(t *testing.T) { CIDR: "1.1.0.0/24", }, }, - Ports: []networking.NetworkPolicyPort{ + Ports: []NetworkPolicyPort{ { Protocol: &tcp, - Port: &invalidPort, + Port: invalidPort, }, }, }, diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 24698576..7591e965 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -5,6 +5,7 @@ package v1 import ( + corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" @@ -74,7 +75,7 @@ func (in *EgressRule) DeepCopyInto(out *EgressRule) { *out = *in if in.Ports != nil { in, out := &in.Ports, &out.Ports - *out = make([]networkingv1.NetworkPolicyPort, len(*in)) + *out = make([]NetworkPolicyPort, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -183,7 +184,7 @@ func (in *IngressRule) DeepCopyInto(out *IngressRule) { *out = *in if in.Ports != nil { in, out := &in.Ports, &out.Ports - *out = make([]networkingv1.NetworkPolicyPort, len(*in)) + *out = make([]NetworkPolicyPort, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -207,6 +208,31 @@ func (in *IngressRule) DeepCopy() *IngressRule { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NetworkPolicyPort) DeepCopyInto(out *NetworkPolicyPort) { + *out = *in + if in.Protocol != nil { + in, out := &in.Protocol, &out.Protocol + *out = new(corev1.Protocol) + **out = **in + } + if in.EndPort != nil { + in, out := &in.EndPort, &out.EndPort + *out = new(int32) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NetworkPolicyPort. +func (in *NetworkPolicyPort) DeepCopy() *NetworkPolicyPort { + if in == nil { + return nil + } + out := new(NetworkPolicyPort) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PolicySpec) DeepCopyInto(out *PolicySpec) { *out = *in diff --git a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml index 2b88b981..18ac0e7c 100644 --- a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml +++ b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml @@ -80,23 +80,17 @@ spec: description: |- endPort indicates that the range of ports from port to endPort if set, inclusive, should be allowed by the policy. This field cannot be defined if the port field - is not defined or if the port field is defined as a named (string) port. + is not defined. The endPort must be equal or greater than port. format: int32 type: integer port: - anyOf: - - type: integer - - type: string - description: |- - port represents the port on the given protocol. This can either be a numerical or named - port on a pod. If this field is not provided, this matches all port names and - numbers. - If present, only traffic on the specified protocol AND port will be matched. - x-kubernetes-int-or-string: true + description: port represents the port on the given protocol. + format: int32 + type: integer protocol: description: |- - protocol represents the protocol (TCP, UDP, or SCTP) which traffic must match. + protocol represents the protocol (TCP, UDP) which traffic must match. If not specified, this field defaults to TCP. type: string type: object @@ -213,23 +207,17 @@ spec: description: |- endPort indicates that the range of ports from port to endPort if set, inclusive, should be allowed by the policy. This field cannot be defined if the port field - is not defined or if the port field is defined as a named (string) port. + is not defined. The endPort must be equal or greater than port. format: int32 type: integer port: - anyOf: - - type: integer - - type: string - description: |- - port represents the port on the given protocol. This can either be a numerical or named - port on a pod. If this field is not provided, this matches all port names and - numbers. - If present, only traffic on the specified protocol AND port will be matched. - x-kubernetes-int-or-string: true + description: port represents the port on the given protocol. + format: int32 + type: integer protocol: description: |- - protocol represents the protocol (TCP, UDP, or SCTP) which traffic must match. + protocol represents the protocol (TCP, UDP) which traffic must match. If not specified, this field defaults to TCP. type: string type: object diff --git a/controllers/firewall_controller_test.go b/controllers/firewall_controller_test.go index d0f5f566..439f5485 100644 --- a/controllers/firewall_controller_test.go +++ b/controllers/firewall_controller_test.go @@ -62,9 +62,9 @@ func TestConvert(t *testing.T) { Spec: firewallv1.PolicySpec{ Egress: []firewallv1.EgressRule{ { - Ports: []networking.NetworkPolicyPort{ + Ports: []firewallv1.NetworkPolicyPort{ { - Port: &p, + Port: p.IntVal, Protocol: &tcp, }, }, @@ -166,8 +166,18 @@ func convert(np networking.NetworkPolicy) (*firewallv1.ClusterwideNetworkPolicy, if len(newTos) == 0 { continue } + + var ports []firewallv1.NetworkPolicyPort + for _, p := range egress.Ports { + ports = append(ports, firewallv1.NetworkPolicyPort{ + Protocol: p.Protocol, + Port: p.Port.IntVal, + EndPort: p.EndPort, + }) + } + newEgresses = append(newEgresses, firewallv1.EgressRule{ - Ports: egress.Ports, + Ports: ports, To: newTos, }) } diff --git a/pkg/nftables/networkpolicy.go b/pkg/nftables/networkpolicy.go index 85b56812..4e13de38 100644 --- a/pkg/nftables/networkpolicy.go +++ b/pkg/nftables/networkpolicy.go @@ -2,10 +2,9 @@ package nftables import ( "fmt" + "strconv" "strings" - networkingv1 "k8s.io/api/networking/v1" - firewallv1 "github.com/metal-stack/firewall-controller/v2/api/v1" ) @@ -150,19 +149,26 @@ func clusterwideNetworkPolicyEgressToFQDNRules( return rules, fqdnState } -func calculatePorts(ports []networkingv1.NetworkPolicyPort) (tcpPorts, udpPorts []string) { +func calculatePorts(ports []firewallv1.NetworkPolicyPort) (tcpPorts, udpPorts []string) { for _, p := range ports { - proto := proto(p.Protocol) - portStr := fmt.Sprint(p.Port) + var ( + proto = proto(p.Protocol) + portStr = strconv.FormatInt(int64(p.Port), 10) + ) + if p.EndPort != nil { - portStr = fmt.Sprintf("%s-%d", p.Port, *p.EndPort) + portStr = fmt.Sprintf("%s-%d", portStr, *p.EndPort) } + switch proto { - case "tcp": - tcpPorts = append(tcpPorts, portStr) case "udp": udpPorts = append(udpPorts, portStr) + case "tcp": + fallthrough + default: + tcpPorts = append(tcpPorts, portStr) } } + return tcpPorts, udpPorts } diff --git a/pkg/nftables/networkpolicy_test.go b/pkg/nftables/networkpolicy_test.go index fe4bcea6..5590ed67 100644 --- a/pkg/nftables/networkpolicy_test.go +++ b/pkg/nftables/networkpolicy_test.go @@ -6,17 +6,11 @@ import ( "github.com/google/go-cmp/cmp" corev1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1" - "k8s.io/apimachinery/pkg/util/intstr" firewallv1 "github.com/metal-stack/firewall-controller/v2/api/v1" mocks "github.com/metal-stack/firewall-controller/v2/pkg/nftables/mocks/pkg/nftables" ) -func port(p int) *intstr.IntOrString { - intstr := intstr.FromInt(p) - return &intstr -} - func TestClusterwideNetworkPolicyRules(t *testing.T) { tcp := corev1.ProtocolTCP udp := corev1.ProtocolUDP @@ -48,18 +42,18 @@ func TestClusterwideNetworkPolicyRules(t *testing.T) { CIDR: "1.1.1.0/24", }, }, - Ports: []networking.NetworkPolicyPort{ + Ports: []firewallv1.NetworkPolicyPort{ { Protocol: &tcp, - Port: port(53), + Port: int32(53), }, { Protocol: &udp, - Port: port(53), + Port: int32(53), }, { Protocol: &tcp, - Port: port(443), + Port: int32(443), EndPort: new(int32(448)), }, }, @@ -73,14 +67,14 @@ func TestClusterwideNetworkPolicyRules(t *testing.T) { Except: []string{"1.1.0.1"}, }, }, - Ports: []networking.NetworkPolicyPort{ + Ports: []firewallv1.NetworkPolicyPort{ { Protocol: &tcp, - Port: port(80), + Port: int32(80), }, { Protocol: &tcp, - Port: port(443), + Port: int32(443), EndPort: new(int32(448)), }, }, @@ -157,14 +151,14 @@ func TestClusterwideNetworkPolicyEgressRules(t *testing.T) { CIDR: "1.1.1.0/24", }, }, - Ports: []networking.NetworkPolicyPort{ + Ports: []firewallv1.NetworkPolicyPort{ { Protocol: &tcp, - Port: port(53), + Port: int32(53), }, { Protocol: &udp, - Port: port(53), + Port: int32(53), }, }, }, @@ -197,14 +191,14 @@ func TestClusterwideNetworkPolicyEgressRules(t *testing.T) { MatchPattern: "*.test.com", }, }, - Ports: []networking.NetworkPolicyPort{ + Ports: []firewallv1.NetworkPolicyPort{ { Protocol: &tcp, - Port: port(53), + Port: int32(53), }, { Protocol: &udp, - Port: port(53), + Port: int32(53), }, }, }, diff --git a/pkg/nftables/service_test.go b/pkg/nftables/service_test.go index a6c7b6c6..bae4e2f9 100644 --- a/pkg/nftables/service_test.go +++ b/pkg/nftables/service_test.go @@ -8,6 +8,7 @@ import ( "go4.org/netipx" corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" ) func helpMustParseIPSet(ips []string) *netipx.IPSet { @@ -15,6 +16,11 @@ func helpMustParseIPSet(ips []string) *netipx.IPSet { return res } +func port(p int) *intstr.IntOrString { + intstr := intstr.FromInt(p) + return &intstr +} + func TestServiceRules(t *testing.T) { type want struct { ingress nftablesRules diff --git a/pkg/nftables/snat_test.go b/pkg/nftables/snat_test.go index f7837fb4..8b9bde97 100644 --- a/pkg/nftables/snat_test.go +++ b/pkg/nftables/snat_test.go @@ -8,7 +8,6 @@ import ( "github.com/google/go-cmp/cmp" mn "github.com/metal-stack/metal-lib/pkg/net" corev1 "k8s.io/api/core/v1" - networking "k8s.io/api/networking/v1" firewallv2 "github.com/metal-stack/firewall-controller-manager/api/v2" firewallv1 "github.com/metal-stack/firewall-controller/v2/api/v1" @@ -132,14 +131,14 @@ func TestSnatRules(t *testing.T) { MatchPattern: "*.test.com", }, }, - Ports: []networking.NetworkPolicyPort{ + Ports: []firewallv1.NetworkPolicyPort{ { Protocol: &tcp, - Port: port(53), + Port: int32(53), }, { Protocol: &udp, - Port: port(53), + Port: int32(53), }, }, },