update

Signed-off-by: Hang Yan <[email protected]>

Author: hangyan

pkg/antctl/antctl.go

@@ -753,7 +753,7 @@ $ antctl get podmulticaststats pod -n namespace`,
 		},
 		{
 			cobraCommand:      packetcapture.Command,
-			supportAgent:      true,
+			supportAgent:      false,
 			supportController: true,
 		},
 		{

pkg/antctl/command_list_test.go

@@ -70,7 +70,7 @@ func TestGetDebugCommands(t *testing.T) {
 		{
 			name:     "Antctl running against agent mode",
 			mode:     "agent",
-			expected: [][]string{{"version"}, {"get", "podmulticaststats"}, {"log-level"}, {"get", "networkpolicy"}, {"get", "appliedtogroup"}, {"get", "addressgroup"}, {"get", "agentinfo"}, {"get", "podinterface"}, {"get", "ovsflows"}, {"trace-packet"}, {"get", "serviceexternalip"}, {"get", "memberlist"}, {"get", "bgppolicy"}, {"get", "bgppeers"}, {"get", "bgproutes"}, {"supportbundle"}, {"traceflow"}, {"packetcapture"}, {"get", "featuregates"}},
+			expected: [][]string{{"version"}, {"get", "podmulticaststats"}, {"log-level"}, {"get", "networkpolicy"}, {"get", "appliedtogroup"}, {"get", "addressgroup"}, {"get", "agentinfo"}, {"get", "podinterface"}, {"get", "ovsflows"}, {"trace-packet"}, {"get", "serviceexternalip"}, {"get", "memberlist"}, {"get", "bgppolicy"}, {"get", "bgppeers"}, {"get", "bgproutes"}, {"supportbundle"}, {"traceflow"}, {"get", "featuregates"}},
 		},
 		{
 			name:     "Antctl running against flow-aggregator mode",

pkg/antctl/raw/packetcapture/command.go

@@ -48,6 +48,7 @@ var (
 	getCopier      = getPodFile
 	defaultFS      = afero.NewOsFs()
 )
+
 var option = &struct {
 	source    string
 	dest      string
@@ -59,11 +60,11 @@ var option = &struct {
 }{}
 
 var packetCaptureExample = strings.TrimSpace(`
-  Start capture packets from pod1 to pod2, both Pods are in Namespace default
+  Start capturing packets from pod1 to pod2, both Pods are in Namespace default
   $ antctl packetcaputre -S pod1 -D pod2
-  Start capture packets from pod1 in Namespace ns1 to a destination IP
+  Start capturing packets from pod1 in Namespace ns1 to a destination IP
   $ antctl packetcapture -S ns1/pod1 -D 192.168.123.123
-  Start capture UDP packets from pod1 to pod2, with destination port 1234
+  Start capturing UDP packets from pod1 to pod2, with destination port 1234
   $ antctl packetcapture -S pod1 -D pod2 -f udp,udp_dst=1234
   Save the packets file to a specified directory
   $ antctl packetcapture -S 192.168.123.123 -D pod2 -f tcp,tcp_dst=80 -o /tmp
@@ -114,6 +115,17 @@ func getFlowFields(flow string) (map[string]int, error) {
 	return fields, nil
 }
 
+func getPodFile(cmd *cobra.Command) (PodFileCopy, error) {
+	config, client, _, err := getClients(cmd)
+	if err != nil {
+		return nil, err
+	}
+	return &podFile{
+		restConfig: config,
+		client:     client,
+	}, nil
+}
+
 func getConfigAndClients(cmd *cobra.Command) (*rest.Config, kubernetes.Interface, antrea.Interface, error) {
 	kubeConfig, err := raw.ResolveKubeconfig(cmd)
 	if err != nil {
@@ -126,20 +138,20 @@ func getConfigAndClients(cmd *cobra.Command) (*rest.Config, kubernetes.Interface
 	return kubeConfig, k8sClientset, client, nil
 }
 
-func getPodFile(cmd *cobra.Command) (PodFileCopy, error) {
-	config, client, _, err := getClients(cmd)
-	if err != nil {
-		return nil, err
+func getPCName(src, dest string) string {
+	replace := func(s string) string {
+		return strings.ReplaceAll(s, "/", "-")
 	}
-	return &podFile{
-		restConfig:    config,
-		restInterface: client.CoreV1().RESTClient(),
-	}, nil
+	prefix := fmt.Sprintf("%s-%s", replace(src), replace(dest))
+	if option.nowait {
+		return prefix
+	}
+	return fmt.Sprintf("%s-%s", prefix, rand.String(8))
 }
 
 func packetCaptureRunE(cmd *cobra.Command, args []string) error {
 	option.timeout, _ = cmd.Flags().GetDuration("timeout")
-	if option.timeout > time.Hour {
+	if option.timeout > 300*time.Second {
 		return errors.New("timeout cannot be longer than 1 hour")
 	}
 	if option.timeout == 0 {
@@ -157,7 +169,7 @@ func packetCaptureRunE(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return fmt.Errorf("error when constructing a PacketCapture CR: %w", err)
 	}
-	createCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	createCtx, cancel := context.WithTimeout(cmd.Context(), 5*time.Second)
 	defer cancel()
 
 	if _, err := antreaClient.CrdV1alpha1().PacketCaptures().Create(createCtx, pc, metav1.CreateOptions{}); err != nil {
@@ -189,7 +201,6 @@ func packetCaptureRunE(cmd *cobra.Command, args []string) error {
 			}
 		}
 		return false, nil
-
 	})
 
 	if wait.Interrupted(err) {
@@ -204,14 +215,16 @@ func packetCaptureRunE(cmd *cobra.Command, args []string) error {
 	splits := strings.Split(latestPC.Status.FilePath, ":")
 	fileName := filepath.Base(splits[1])
 	copier, _ := getCopier(cmd)
-	err = copier.CopyFromPod(context.TODO(), env.GetAntreaNamespace(), splits[0], "antrea-agent", splits[1], option.outputDir)
-	if err == nil {
-		fmt.Fprintf(cmd.OutOrStdout(), "Captured packets file: %s\n", filepath.Join(option.outputDir, fileName))
+	if err := copier.CopyFromPod(cmd.Context(), env.GetAntreaNamespace(), splits[0], "antrea-agent", splits[1], option.outputDir); err != nil {
+		return err
 	}
-	return err
+	fmt.Fprintf(cmd.OutOrStdout(), "Captured packets file: %s\n", filepath.Join(option.outputDir, fileName))
+	return nil
 }
 
-func parseEndpoint(endpoint string) (pod *v1alpha1.PodReference, ip *string) {
+func parseEndpoint(endpoint string) (*v1alpha1.PodReference, *string) {
+	var pod *v1alpha1.PodReference
+	var ip *string
 	parsedIP := net.ParseIP(endpoint)
 	if parsedIP != nil && parsedIP.To4() != nil {
 		ip = ptr.To(parsedIP.String())
@@ -229,23 +242,12 @@ func parseEndpoint(endpoint string) (pod *v1alpha1.PodReference, ip *string) {
 			}
 		}
 	}
-	return
-}
-
-func getPCName(src, dest string) string {
-	replace := func(s string) string {
-		return strings.ReplaceAll(s, "/", "-")
-	}
-	prefix := fmt.Sprintf("%s-%s", replace(src), replace(dest))
-	if option.nowait {
-		return prefix
-	}
-	return fmt.Sprintf("%s-%s", prefix, rand.String(8))
+	return pod, ip
 }
 
 func parseFlow() (*v1alpha1.Packet, error) {
 	trimFlow := strings.ReplaceAll(option.flow, " ", "")
-	fields, err := getFlowFields(cleanFlow)
+	fields, err := getFlowFields(trimFlow)
 	if err != nil {
 		return nil, fmt.Errorf("error when parsing the flow: %w", err)
 	}

pkg/antctl/raw/packetcapture/command_test.go

@@ -110,14 +110,14 @@ func TestRun(t *testing.T) {
 			src:       srcPod,
 			dst:       dstPod,
 			flow:      "icmp",
-			expectErr: "timeout waiting for PacketCapture done",
+			expectErr: "timeout while waiting for PacketCapture to complete",
 		},
 		{
 			name:      "invalid-packetcapture",
 			src:       ipv4,
 			dst:       ipv4,
 			flow:      "icmp",
-			expectErr: "error when filling up PacketCapture config: one of source and destination must be a Pod",
+			expectErr: "error when constructing a PacketCapture CR: one of source and destination must be a Pod",
 		},
 	}
 	for _, tt := range tcs {
@@ -150,7 +150,6 @@ func TestRun(t *testing.T) {
 			getCopier = func(cmd *cobra.Command) (PodFileCopy, error) {
 				return &testPodFile{}, nil
 			}
-
 			defer func() {
 				getClients = getConfigAndClients
 				getCopier = getPodFile
@@ -159,6 +158,7 @@ func TestRun(t *testing.T) {
 			Command.SetOutput(buf)
 			Command.SetOut(buf)
 			Command.SetErr(buf)
+			Command.SetContext(context.Background())
 
 			err := packetCaptureRunE(Command, nil)
 			if tt.expectErr == "" {

pkg/antctl/raw/packetcapture/cp.go

@@ -16,61 +16,33 @@ package packetcapture
 
 import (
 	"context"
-	"io"
-	"os"
-	_ "unsafe"
+	"fmt"
+	"path/filepath"
+	"strings"
 
-	corev1 "k8s.io/api/core/v1"
-	"k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
-	"k8s.io/client-go/tools/remotecommand"
 
+	"antrea.io/antrea/pkg/antctl/raw/check"
 	"antrea.io/antrea/pkg/util/compress"
+	"antrea.io/antrea/pkg/util/env"
 )
 
 type PodFileCopy interface {
 	CopyFromPod(ctx context.Context, namespace, name, containerName, srcPath, dstDir string) error
 }
 
 type podFile struct {
-	restConfig    *rest.Config
-	restInterface rest.Interface
+	restConfig *rest.Config
+	client     kubernetes.Interface
 }
 
 func (p *podFile) CopyFromPod(ctx context.Context, namespace, name, containerName, srcPath, dstDir string) error {
-	reader, outStream := io.Pipe()
-	cmdArr := []string{"tar", "cf", "-", srcPath}
-	req := p.restInterface.
-		Get().
-		Namespace(namespace).
-		Resource("pods").
-		Name(name).
-		SubResource("exec").
-		VersionedParams(&corev1.PodExecOptions{
-			Container: containerName,
-			Command:   cmdArr,
-			Stdin:     true,
-			Stdout:    true,
-			Stderr:    true,
-			TTY:       false,
-		}, scheme.ParameterCodec)
-
-	exec, err := remotecommand.NewSPDYExecutor(p.restConfig, "POST", req.URL())
+	dir, fileName := filepath.Split(srcPath)
+	cmdArr := []string{"/bin/sh", "-c", fmt.Sprintf("cd %s; tar cf - %s", dir, fileName)}
+	output, _, err := check.ExecInPod(ctx, p.client, p.restConfig, env.GetAntreaNamespace(), name, "antrea-agent", cmdArr)
 	if err != nil {
 		return err
 	}
-	go func() {
-		defer outStream.Close()
-		err = exec.StreamWithContext(ctx, remotecommand.StreamOptions{
-			Stdin:  os.Stdin,
-			Stdout: outStream,
-			Stderr: os.Stderr,
-			Tty:    false,
-		})
-		if err != nil {
-			panic(err)
-		}
-	}()
-	err = compress.UnpackReader(defaultFS, reader, dstDir)
-	return err
+	return compress.UnpackReader(defaultFS, strings.NewReader(output), false, option.outputDir)
 }

pkg/util/compress/compress.go

@@ -31,7 +31,7 @@ import (
 // Sanitize archive file pathing from "G305: Zip Slip vulnerability"
 func sanitizeArchivePath(d, t string) (string, error) {
 	v := filepath.Join(d, t)
-	if strings.HasPrefix(v, filepath.Clean(d)) {
+	if strings.HasPrefix(v, filepath.Clean(d)) || (filepath.Clean(d) == "." && v == t) {
 		return v, nil
 	}
 	return "", fmt.Errorf("%s: %s", "content filepath is tainted", t)
@@ -43,15 +43,21 @@ func UnpackDir(fs afero.Fs, fileName string, targetDir string) error {
 		return err
 	}
 	defer file.Close()
-	return UnpackReader(fs, file, targetDir)
+	return UnpackReader(fs, file, true, targetDir)
 }
 
-func UnpackReader(fs afero.Fs, file io.Reader, targetDir string) error {
-	reader, err := gzip.NewReader(file)
-	if err != nil {
-		return err
+func UnpackReader(fs afero.Fs, file io.Reader, useGzip bool, targetDir string) error {
+	reader := file
+	var err error
+	var gzipReader *gzip.Reader
+	if useGzip {
+		gzipReader, err = gzip.NewReader(file)
+		if err != nil {
+			return err
+		}
+		defer gzipReader.Close()
+		reader = gzipReader
 	}
-	defer reader.Close()
 	tarReader := tar.NewReader(reader)
 
 	for true {
@@ -73,10 +79,10 @@ func UnpackReader(fs afero.Fs, file io.Reader, targetDir string) error {
 			}
 		case tar.TypeReg:
 			outFile, err := fs.Create(targetPath)
-			defer outFile.Close()
 			if err != nil {
 				return err
 			}
+			defer outFile.Close()
 			for {
 				// to resolve G110: Potential DoS vulnerability via decompression bomb
 				if _, err := io.CopyN(outFile, tarReader, 1024); err != nil {