From 13d3d42022299a4eb6e68d229fcb42c6b6d69b91 Mon Sep 17 00:00:00 2001 From: Hang Yan Date: Sat, 14 Sep 2024 16:16:14 +0800 Subject: [PATCH] fix unit tests and api updates Signed-off-by: Hang Yan --- .../packetcapture/packetcapture_controller.go | 70 ++++++++----------- .../packetcapture_controller_test.go | 70 +++++++++++++------ .../controller/packetcapture/packetin_test.go | 2 +- .../crd/v1alpha1/zz_generated.deepcopy.go | 7 -- pkg/features/antrea_features.go | 2 +- test/e2e/packetcapture_test.go | 10 +-- 6 files changed, 85 insertions(+), 76 deletions(-) diff --git a/pkg/agent/controller/packetcapture/packetcapture_controller.go b/pkg/agent/controller/packetcapture/packetcapture_controller.go index 9257a70e9ea..071d08d88a9 100644 --- a/pkg/agent/controller/packetcapture/packetcapture_controller.go +++ b/pkg/agent/controller/packetcapture/packetcapture_controller.go @@ -393,15 +393,15 @@ func (c *Controller) startPacketCapture(pc *crdv1alpha1.PacketCapture, pcState * return err } -// genEndpointMatchPackets generates match packets (with destination Endpoint's IP/port info) besides the normal match packet. +// genEndpointMatchPackets generates match packets (with destination Endpoint's IP/Port info) besides the normal match packet. // these match packets will help the pipeline to capture the pod -> svc traffic. // TODO: 1. support name based port name 2. dual-stack support func (c *Controller) genEndpointMatchPackets(pc *crdv1alpha1.PacketCapture) ([]binding.Packet, error) { var port int32 - if pc.Spec.Packet.TransportHeader.TCP != nil { - port = pc.Spec.Packet.TransportHeader.TCP.DstPort - } else if pc.Spec.Packet.TransportHeader.UDP != nil { - port = pc.Spec.Packet.TransportHeader.UDP.DstPort + if pc.Spec.Packet.TransportHeader.TCP != nil && pc.Spec.Packet.TransportHeader.TCP.DstPort != nil { + port = *pc.Spec.Packet.TransportHeader.TCP.DstPort + } else if pc.Spec.Packet.TransportHeader.UDP != nil && pc.Spec.Packet.TransportHeader.UDP.DstPort != nil { + port = *pc.Spec.Packet.TransportHeader.UDP.DstPort } var packets []binding.Packet dstSvc, err := c.serviceLister.Services(pc.Spec.Destination.Namespace).Get(pc.Spec.Destination.Service) @@ -425,7 +425,7 @@ func (c *Controller) genEndpointMatchPackets(pc *crdv1alpha1.PacketCapture) ([]b if port != 0 { packet.DestinationPort = uint16(port) } - packet.IPProto, _ = parseTargetProto(pc.Spec.Packet) + packet.IPProto = parseTargetProto(pc.Spec.Packet) packets = append(packets, packet) } return packets, nil @@ -497,55 +497,45 @@ func (c *Controller) preparePacket(pc *crdv1alpha1.PacketCapture, intf *interfac } if pc.Spec.Packet.TransportHeader.TCP != nil { - packet.SourcePort = uint16(pc.Spec.Packet.TransportHeader.TCP.SrcPort) - packet.DestinationPort = uint16(pc.Spec.Packet.TransportHeader.TCP.DstPort) + if pc.Spec.Packet.TransportHeader.TCP.SrcPort != nil { + packet.SourcePort = uint16(*pc.Spec.Packet.TransportHeader.TCP.SrcPort) + } + if pc.Spec.Packet.TransportHeader.TCP.DstPort != nil { + packet.DestinationPort = uint16(*pc.Spec.Packet.TransportHeader.TCP.DstPort) + } if pc.Spec.Packet.TransportHeader.TCP.Flags != nil { packet.TCPFlags = uint8(*pc.Spec.Packet.TransportHeader.TCP.Flags) } } else if pc.Spec.Packet.TransportHeader.UDP != nil { - packet.SourcePort = uint16(pc.Spec.Packet.TransportHeader.UDP.SrcPort) - packet.DestinationPort = uint16(pc.Spec.Packet.TransportHeader.UDP.DstPort) - } - - proto, err := parseTargetProto(pc.Spec.Packet) - if err != nil { - return nil, err + if pc.Spec.Packet.TransportHeader.UDP.SrcPort != nil { + packet.SourcePort = uint16(*pc.Spec.Packet.TransportHeader.UDP.SrcPort) + } + if pc.Spec.Packet.TransportHeader.UDP.DstPort != nil { + packet.DestinationPort = uint16(*pc.Spec.Packet.TransportHeader.UDP.DstPort) + } } - packet.IPProto = proto + packet.IPProto = parseTargetProto(pc.Spec.Packet) return packet, nil } -func parseTargetProto(packet *crdv1alpha1.Packet) (uint8, error) { - var ipProto uint8 - isIPv6 := packet.IPFamily == v1.IPv6Protocol - - if packet.TransportHeader.TCP != nil { - ipProto = protocol.Type_TCP - } else if packet.TransportHeader.UDP != nil { - ipProto = protocol.Type_UDP - } else { - ipProto = protocol.Type_ICMP - if isIPv6 { - ipProto = protocol.Type_IPv6ICMP - } +func parseTargetProto(packet *crdv1alpha1.Packet) uint8 { + inputProto := packet.Protocol + if inputProto == nil { + return protocol.Type_ICMP + } + if inputProto.Type == intstr.Int { + return uint8(inputProto.IntVal) } - inputProto := packet.Protocol if inputProto.StrVal == "TCP" { - inputProto = &intstr.IntOrString{Type: intstr.Int, IntVal: protocol.Type_TCP} + return protocol.Type_TCP } else if inputProto.StrVal == "ICMP" { - inputProto = &intstr.IntOrString{Type: intstr.Int, IntVal: protocol.Type_ICMP} + return protocol.Type_ICMP } else if inputProto.StrVal == "UDP" { - inputProto = &intstr.IntOrString{Type: intstr.Int, IntVal: protocol.Type_UDP} + return protocol.Type_UDP } else { - inputProto = &intstr.IntOrString{Type: intstr.Int, IntVal: protocol.Type_IPv6ICMP} - } - - if inputProto.IntVal != int32(ipProto) { - return 0, errors.New("Unmatch protocol settings in Packet between transportHeader and protocol") + return protocol.Type_IPv6ICMP } - - return ipProto, nil } func (c *Controller) syncPacketCapture(pcName string) error { diff --git a/pkg/agent/controller/packetcapture/packetcapture_controller_test.go b/pkg/agent/controller/packetcapture/packetcapture_controller_test.go index c16e12786ef..a29620e2450 100644 --- a/pkg/agent/controller/packetcapture/packetcapture_controller_test.go +++ b/pkg/agent/controller/packetcapture/packetcapture_controller_test.go @@ -51,16 +51,20 @@ import ( ) var ( - pod1IPv4 = "192.168.10.10" - pod2IPv4 = "192.168.11.10" - service1IPv4 = "10.96.0.10" - dstIPv4 = "192.168.99.99" - pod1MAC, _ = net.ParseMAC("aa:bb:cc:dd:ee:0f") - pod2MAC, _ = net.ParseMAC("aa:bb:cc:dd:ee:00") - ofPortPod1 = uint32(1) - ofPortPod2 = uint32(2) - testTCPFlags = int32(11) - icmp6Proto = intstr.FromInt(58) + pod1IPv4 = "192.168.10.10" + pod2IPv4 = "192.168.11.10" + service1IPv4 = "10.96.0.10" + dstIPv4 = "192.168.99.99" + pod1MAC, _ = net.ParseMAC("aa:bb:cc:dd:ee:0f") + pod2MAC, _ = net.ParseMAC("aa:bb:cc:dd:ee:00") + ofPortPod1 = uint32(1) + ofPortPod2 = uint32(2) + testTCPFlags = int32(11) + icmp6Proto = intstr.FromInt(58) + tcpProto = intstr.FromString("TCP") + icmpProto = intstr.FromString("ICMP") + port80 int32 = 80 + port81 int32 = 81 pod1 = v1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -249,10 +253,11 @@ func TestPreparePacket(t *testing.T) { Pod: pod2.Name, }, Packet: &crdv1alpha1.Packet{ + Protocol: &intstr.IntOrString{Type: intstr.String, StrVal: "TCP"}, TransportHeader: crdv1alpha1.TransportHeader{ TCP: &crdv1alpha1.TCPHeader{ - SrcPort: 80, - DstPort: 81, + SrcPort: &port80, + DstPort: &port81, Flags: &testTCPFlags, }, }, @@ -279,6 +284,10 @@ func TestPreparePacket(t *testing.T) { Namespace: pod1.Namespace, Pod: pod1.Name, }, + Packet: &crdv1alpha1.Packet{ + IPFamily: v1.IPv4Protocol, + Protocol: &intstr.IntOrString{Type: intstr.String, StrVal: "ICMP"}, + }, }, }, receiverOnly: true, @@ -303,6 +312,7 @@ func TestPreparePacket(t *testing.T) { }, Packet: &crdv1alpha1.Packet{ IPFamily: v1.IPv6Protocol, + Protocol: &icmp6Proto, }, }, }, @@ -346,10 +356,11 @@ func TestPreparePacket(t *testing.T) { Pod: pod2.Name, }, Packet: &crdv1alpha1.Packet{ + Protocol: &intstr.IntOrString{Type: intstr.String, StrVal: "TCP"}, TransportHeader: crdv1alpha1.TransportHeader{ TCP: &crdv1alpha1.TCPHeader{ - SrcPort: 80, - DstPort: 81, + SrcPort: &port80, + DstPort: &port81, }, }, }, @@ -376,10 +387,11 @@ func TestPreparePacket(t *testing.T) { Pod: pod2.Name, }, Packet: &crdv1alpha1.Packet{ + Protocol: &intstr.IntOrString{Type: intstr.String, StrVal: "UDP"}, TransportHeader: crdv1alpha1.TransportHeader{ UDP: &crdv1alpha1.UDPHeader{ - SrcPort: 80, - DstPort: 100, + SrcPort: &port80, + DstPort: &port81, }, }, }, @@ -389,7 +401,7 @@ func TestPreparePacket(t *testing.T) { DestinationIP: net.ParseIP(pod2IPv4), IPProto: protocol.Type_UDP, SourcePort: 80, - DestinationPort: 100, + DestinationPort: 81, }, }, { @@ -405,7 +417,9 @@ func TestPreparePacket(t *testing.T) { Namespace: pod2.Namespace, Pod: pod2.Name, }, - Packet: &crdv1alpha1.Packet{}, + Packet: &crdv1alpha1.Packet{ + Protocol: &intstr.IntOrString{Type: intstr.String, StrVal: "ICMP"}, + }, }, }, expectedPacket: &binding.Packet{ @@ -440,10 +454,11 @@ func TestPreparePacket(t *testing.T) { Namespace: service1.Namespace, }, Packet: &crdv1alpha1.Packet{ + Protocol: &intstr.IntOrString{Type: intstr.String, StrVal: "TCP"}, TransportHeader: crdv1alpha1.TransportHeader{ TCP: &crdv1alpha1.TCPHeader{ - SrcPort: 80, - DstPort: 81, + SrcPort: &port80, + DstPort: &port81, Flags: &testTCPFlags, }, }, @@ -617,6 +632,9 @@ func TestPacketCaptureControllerRun(t *testing.T) { Number: 5, }, }, + Packet: &crdv1alpha1.Packet{ + Protocol: &icmpProto, + }, }, }, newState: &packetCaptureState{tag: 1}, @@ -663,6 +681,9 @@ func TestProcessPacketCaptureItem(t *testing.T) { Number: 5, }, }, + Packet: &crdv1alpha1.Packet{ + Protocol: &icmpProto, + }, }, }, ofPort: ofPortPod1, @@ -836,9 +857,10 @@ func TestPrepareEndpointsPackets(t *testing.T) { Service: "svc1", }, Packet: &crdv1alpha1.Packet{ + Protocol: &intstr.IntOrString{Type: intstr.String, StrVal: "TCP"}, TransportHeader: crdv1alpha1.TransportHeader{ TCP: &crdv1alpha1.TCPHeader{ - DstPort: 80, + DstPort: &port80, }, }, }, @@ -878,9 +900,10 @@ func TestPrepareEndpointsPackets(t *testing.T) { Service: "svc1", }, Packet: &crdv1alpha1.Packet{ + Protocol: &intstr.IntOrString{Type: intstr.String, StrVal: "TCP"}, TransportHeader: crdv1alpha1.TransportHeader{ TCP: &crdv1alpha1.TCPHeader{ - DstPort: 80, + DstPort: &port80, }, }, }, @@ -954,9 +977,10 @@ func TestPrepareEndpointsPackets(t *testing.T) { Service: "svc1", }, Packet: &crdv1alpha1.Packet{ + Protocol: &intstr.IntOrString{Type: intstr.String, StrVal: "TCP"}, TransportHeader: crdv1alpha1.TransportHeader{ TCP: &crdv1alpha1.TCPHeader{ - DstPort: 80, + DstPort: &port80, }, }, }, diff --git a/pkg/agent/controller/packetcapture/packetin_test.go b/pkg/agent/controller/packetcapture/packetin_test.go index e5d21138246..5b3c10b7268 100644 --- a/pkg/agent/controller/packetcapture/packetin_test.go +++ b/pkg/agent/controller/packetcapture/packetin_test.go @@ -256,7 +256,7 @@ func TestHandlePacketCapturePacketIn(t *testing.T) { // check target num in status pc, err := pcc.crdClient.CrdV1alpha1().PacketCaptures().Get(context.TODO(), tt.expectedPC.Name, metav1.GetOptions{}) require.Nil(t, err) - assert.Equal(t, tt.expectedNum, pc.Status.NumCapturedPackets) + assert.Equal(t, tt.expectedNum, *pc.Status.NumCapturedPackets) } else { assert.Equal(t, tt.expectedErrStr, err.Error()) } diff --git a/pkg/apis/crd/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/crd/v1alpha1/zz_generated.deepcopy.go index 74ad718b3fc..f4a512257c4 100644 --- a/pkg/apis/crd/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/crd/v1alpha1/zz_generated.deepcopy.go @@ -963,11 +963,7 @@ func (in *TransportHeader) DeepCopyInto(out *TransportHeader) { if in.UDP != nil { in, out := &in.UDP, &out.UDP *out = new(UDPHeader) -<<<<<<< HEAD (*in).DeepCopyInto(*out) -======= - **out = **in ->>>>>>> 07604b78c (Add packetcapture feature) } if in.TCP != nil { in, out := &in.TCP, &out.TCP @@ -990,7 +986,6 @@ func (in *TransportHeader) DeepCopy() *TransportHeader { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *UDPHeader) DeepCopyInto(out *UDPHeader) { *out = *in -<<<<<<< HEAD if in.SrcPort != nil { in, out := &in.SrcPort, &out.SrcPort *out = new(int32) @@ -1001,8 +996,6 @@ func (in *UDPHeader) DeepCopyInto(out *UDPHeader) { *out = new(int32) **out = **in } -======= ->>>>>>> 07604b78c (Add packetcapture feature) return } diff --git a/pkg/features/antrea_features.go b/pkg/features/antrea_features.go index 54546d6b97c..0c8086f4c8b 100644 --- a/pkg/features/antrea_features.go +++ b/pkg/features/antrea_features.go @@ -73,7 +73,7 @@ const ( // Allows to trace path from a generated packet. Traceflow featuregate.Feature = "Traceflow" - // alpha: v2.0 + // alpha: v2.2 // Allows to capture packets for a flow. PacketCapture featuregate.Feature = "PacketCapture" diff --git a/test/e2e/packetcapture_test.go b/test/e2e/packetcapture_test.go index 923ebaa7e8e..934ab6eff74 100644 --- a/test/e2e/packetcapture_test.go +++ b/test/e2e/packetcapture_test.go @@ -49,6 +49,8 @@ var ( icmpProto = intstr.FromInt(1) udpProto = intstr.FromInt(17) icmp6Proto = intstr.FromInt(58) + + testServerPort int32 = 80 ) type pcTestCase struct { @@ -248,7 +250,7 @@ func testPacketCapture(t *testing.T, data *TestData) { Protocol: &tcpProto, TransportHeader: crdv1alpha1.TransportHeader{ TCP: &crdv1alpha1.TCPHeader{ - DstPort: serverPodPort, + DstPort: &testServerPort, }, }, }, @@ -287,7 +289,7 @@ func testPacketCapture(t *testing.T, data *TestData) { Protocol: &tcpProto, TransportHeader: crdv1alpha1.TransportHeader{ TCP: &crdv1alpha1.TCPHeader{ - DstPort: serverPodPort, + DstPort: &testServerPort, }, }, }, @@ -365,7 +367,7 @@ func testPacketCaptureBasic(t *testing.T, data *TestData) { Protocol: &tcpProto, TransportHeader: crdv1alpha1.TransportHeader{ TCP: &crdv1alpha1.TCPHeader{ - DstPort: serverPodPort, + DstPort: &testServerPort, }, }, }, @@ -403,7 +405,7 @@ func testPacketCaptureBasic(t *testing.T, data *TestData) { Protocol: &udpProto, TransportHeader: crdv1alpha1.TransportHeader{ UDP: &crdv1alpha1.UDPHeader{ - DstPort: serverPodPort, + DstPort: &testServerPort, }, }, },