Skip to content

Commit

Permalink
fix: capture name flag (#275)
Browse files Browse the repository at this point in the history
# 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 <rbtr@users.noreply.github.com>
  • Loading branch information
rbtr authored Apr 16, 2024
1 parent 96d85f8 commit 72bcb20
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 27 deletions.
4 changes: 4 additions & 0 deletions cli/cmd/capture/capture.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"k8s.io/cli-runtime/pkg/genericclioptions"
)

var name string

var capture = &cobra.Command{
Use: "capture",
Short: "capture network traffic",
Expand All @@ -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")
}
7 changes: 2 additions & 5 deletions cli/cmd/capture/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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")
Expand Down
9 changes: 0 additions & 9 deletions cli/cmd/capture/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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")
}
17 changes: 4 additions & 13 deletions cli/cmd/capture/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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)
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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")
}

0 comments on commit 72bcb20

Please sign in to comment.