From 72bcb2073c42defc9ab4ec960f4b9c8bc090d6dc Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Tue, 16 Apr 2024 16:07:37 -0500 Subject: [PATCH] fix: capture name flag (#275) # Description The `-n,--name` flag on `retina capture download` conflicts with the inherited `-n,--namespace` flag from `cli-runtime`. This change removes the shorthand `-n` from the `--name` flag. Additionally, it: - changes the `--name` flag to be "Persistent" on the `capture` command instead of only declared on some subcommands - marks the `--name` flag "Required" and removes unnecessary manual checks that the value is set - respect the `--name` flag during `create` instead of generating a randstring ## Related Issue If this pull request is related to any issue, please mention it here. Additionally, make sure that the issue is assigned to you before submitting this pull request. ## Checklist - [x] I have read the [contributing documentation](https://retina.sh/docs/contributing). - [x] I signed and signed-off the commits (`git commit -S -s ...`). See [this documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification) on signing commits. - [x] I have correctly attributed the author(s) of the code. - [x] I have tested the changes locally. - [x] I have followed the project's style guidelines. - [x] I have updated the documentation, if necessary. - [x] I have added tests, if applicable. ## Screenshots (if applicable) or Testing Completed Please add any relevant screenshots or GIFs to showcase the changes made. ## Additional Notes Add any additional notes or context about the pull request here. --- Please refer to the [CONTRIBUTING.md](../CONTRIBUTING.md) file for more information on how to contribute to this project. Signed-off-by: Evan Baker --- cli/cmd/capture/capture.go | 4 ++++ cli/cmd/capture/create.go | 7 ++----- cli/cmd/capture/delete.go | 9 --------- cli/cmd/capture/download.go | 17 ++++------------- 4 files changed, 10 insertions(+), 27 deletions(-) diff --git a/cli/cmd/capture/capture.go b/cli/cmd/capture/capture.go index 58241b2145..d8eb7d8fe7 100644 --- a/cli/cmd/capture/capture.go +++ b/cli/cmd/capture/capture.go @@ -9,6 +9,8 @@ import ( "k8s.io/cli-runtime/pkg/genericclioptions" ) +var name string + var capture = &cobra.Command{ Use: "capture", Short: "capture network traffic", @@ -18,4 +20,6 @@ func init() { cmd.Retina.AddCommand(capture) configFlags = genericclioptions.NewConfigFlags(true) configFlags.AddFlags(capture.PersistentFlags()) + capture.PersistentFlags().StringVar(&name, "name", "", "The name of the Retina Capture") + _ = capture.MarkPersistentFlagRequired("name") } diff --git a/cli/cmd/capture/create.go b/cli/cmd/capture/create.go index f7b90f6f16..2fa29610b1 100644 --- a/cli/cmd/capture/create.go +++ b/cli/cmd/capture/create.go @@ -22,7 +22,6 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" - utilrand "k8s.io/apimachinery/pkg/util/rand" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/client-go/kubernetes" @@ -174,10 +173,9 @@ func deleteSecret(kubeClient kubernetes.Interface, secretName *string) error { } func createCaptureF(kubeClient kubernetes.Interface) (*retinav1alpha1.Capture, error) { - captureName := fmt.Sprintf("retina-capture-%s", utilrand.String(5)) capture := &retinav1alpha1.Capture{ ObjectMeta: metav1.ObjectMeta{ - Name: captureName, + Name: name, Namespace: namespace, }, Spec: retinav1alpha1.CaptureSpec{ @@ -255,7 +253,7 @@ func createCaptureF(kubeClient kubernetes.Interface) (*retinav1alpha1.Capture, e if len(blobUpload) != 0 { // Mount blob url as secret onto the capture pod for security concern if blob url is not empty. - secretName, err := createSecretFromBlobUpload(kubeClient, blobUpload, captureName) + secretName, err := createSecretFromBlobUpload(kubeClient, blobUpload, name) if err != nil { return nil, err } @@ -398,7 +396,6 @@ func init() { createCapture.Flags().StringVar(&includeFilter, "include-filter", "", "A comma-separated list of IP:Port pairs that are "+ "used to filter capture network packets. Supported formats are IP:Port, IP, Port, *:Port, IP:*") createCapture.Flags().BoolVar(&includeMetadata, "include-metadata", true, "If true, collect static network metadata into capture file") - createCapture.Flags().StringVarP(&namespace, "namespace", "n", "default", "Namespace to host capture job") createCapture.Flags().IntVar(&jobNumLimit, "job-num-limit", 0, "The maximum number of jobs can be created for each capture. 0 means no limit") createCapture.Flags().BoolVar(&nowait, "no-wait", true, "Do not wait for the long-running capture job to finish") createCapture.Flags().BoolVar(&debug, "debug", false, "When debug is true, a customized retina-agent image, determined by the environment variable RETINA_AGENT_IMAGE, is set") diff --git a/cli/cmd/capture/delete.go b/cli/cmd/capture/delete.go index 0beeeed9b3..b672f7c04b 100644 --- a/cli/cmd/capture/delete.go +++ b/cli/cmd/capture/delete.go @@ -6,7 +6,6 @@ package capture import ( "context" "fmt" - "strings" retinacmd "github.com/microsoft/retina/cli/cmd" captureConstants "github.com/microsoft/retina/pkg/capture/constants" @@ -21,8 +20,6 @@ import ( "k8s.io/kubectl/pkg/util/templates" ) -var name string - var deleteExample = templates.Examples(i18n.T(` # Delete the Retina Capture "retina-capture-8v6wd" in namespace "capture" kubectl retina capture delete --name retina-capture-8v6wd --namespace capture @@ -43,10 +40,6 @@ var deleteCapture = &cobra.Command{ return errors.Wrap(err, "") } - if strings.TrimSpace(name) == "" { - return errors.New("capture name is not specified") - } - captureJobSelector := &metav1.LabelSelector{ MatchLabels: map[string]string{ label.CaptureNameLabel: name, @@ -91,6 +84,4 @@ var deleteCapture = &cobra.Command{ func init() { capture.AddCommand(deleteCapture) - deleteCapture.Flags().StringVar(&name, "name", "", "name of the Retina Capture") - deleteCapture.Flags().StringVarP(&namespace, "namespace", "n", "default", "Namespace to host capture job") } diff --git a/cli/cmd/capture/download.go b/cli/cmd/capture/download.go index 94604252f1..227916f33b 100644 --- a/cli/cmd/capture/download.go +++ b/cli/cmd/capture/download.go @@ -18,10 +18,7 @@ import ( const BlobURL = "BLOB_URL" -var ( - ErrEmptyBlobURL = errors.New("BLOB_URL must be set/exported") - captureName string -) +var ErrEmptyBlobURL = errors.New("BLOB_URL must be set/exported") var downloadCapture = &cobra.Command{ Use: "download", @@ -32,12 +29,7 @@ var downloadCapture = &cobra.Command{ return ErrEmptyBlobURL } - bloburl := viper.GetString(BlobURL) - if bloburl == "" { - return ErrEmptyBlobURL - } - - u, err := url.Parse(bloburl) + u, err := url.Parse(blobURL) if err != nil { return errors.Wrapf(err, "failed to parse SAS URL %s", blobURL) } @@ -53,14 +45,14 @@ var downloadCapture = &cobra.Command{ splitPath := strings.SplitN(containerPath, "/", 2) //nolint:gomnd // TODO string splitting probably isn't the right way to parse this URL? containerName := splitPath[0] - params := storage.ListBlobsParameters{Prefix: captureName} + params := storage.ListBlobsParameters{Prefix: name} blobList, err := blobService.GetContainerReference(containerName).ListBlobs(params) if err != nil { return errors.Wrap(err, "failed to list blobstore ") } if len(blobList.Blobs) == 0 { - return errors.Errorf("no blobs found with prefix: %s", captureName) + return errors.Errorf("no blobs found with prefix: %s", name) } for _, v := range blobList.Blobs { @@ -89,5 +81,4 @@ var downloadCapture = &cobra.Command{ func init() { capture.AddCommand(downloadCapture) - downloadCapture.Flags().StringVarP(&captureName, "capture-name", "n", "", "name of capture to download") }