kubectl debug: add tests for Complete,Validate

Kubernetes-commit: 3cfcf3a74fc24e6b4b3f58710fe454d1bc3644cc

pkg/cmd/debug/debug.go

@@ -94,27 +94,26 @@ var nameSuffixFunc = utilrand.String
 
 // DebugOptions holds the options for an invocation of kubectl debug.
 type DebugOptions struct {
-	Args           []string
+	Args            []string
-	ArgsOnly       bool
+	ArgsOnly        bool
-	Attach         bool
+	Attach          bool
-	Container      string
+	Container       string
-	CopyTo         string
+	CopyTo          string
-	Replace        bool
+	Replace         bool
-	Env            []corev1.EnvVar
+	Env             []corev1.EnvVar
-	Image          string
+	Image           string
-	Interactive    bool
+	Interactive     bool
-	Namespace      string
+	Namespace       string
-	TargetNames    []string
+	TargetNames     []string
-	PullPolicy     corev1.PullPolicy
+	PullPolicy      corev1.PullPolicy
-	Quiet          bool
+	Quiet           bool
-	SameNode       bool
+	SameNode        bool
-	ShareProcesses bool
+	ShareProcesses  bool
-	Target         string
+	TargetContainer string
-	TTY            bool
+	TTY             bool
 
 	shareProcessedChanged bool
 
-	builder   *resource.Builder
 	podClient corev1client.PodsGetter
 
 	genericclioptions.IOStreams
@@ -165,7 +164,7 @@ func addDebugFlags(cmd *cobra.Command, opt *DebugOptions) {
 	cmd.Flags().BoolVar(&opt.Quiet, "quiet", opt.Quiet, i18n.T("If true, suppress informational messages."))
 	cmd.Flags().BoolVar(&opt.SameNode, "same-node", opt.SameNode, i18n.T("When used with '--copy-to', schedule the copy of target Pod on the same node."))
 	cmd.Flags().BoolVar(&opt.ShareProcesses, "share-processes", opt.ShareProcesses, i18n.T("When used with '--copy-to', enable process namespace sharing in the copy."))
-	cmd.Flags().StringVar(&opt.Target, "target", "", i18n.T("When debugging a pod, target processes in this container name."))
+	cmd.Flags().StringVar(&opt.TargetContainer, "target", "", i18n.T("When using an ephemeral container, target processes in this container name."))
 	cmd.Flags().BoolVarP(&opt.TTY, "tty", "t", opt.TTY, i18n.T("Allocate a TTY for the debugging container."))
 }
 
@@ -173,7 +172,6 @@ func addDebugFlags(cmd *cobra.Command, opt *DebugOptions) {
 func (o *DebugOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error {
 	var err error
 
-	o.builder = f.NewBuilder()
 	o.PullPolicy = corev1.PullPolicy(cmdutil.GetFlagString(cmd, "image-pull-policy"))
 
 	// Arguments
@@ -205,13 +203,6 @@ func (o *DebugOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st
 		return err
 	}
 
-	// Clientset
-	clientset, err := f.KubernetesClientSet()
-	if err != nil {
-		return fmt.Errorf("internal error getting clientset: %v", err)
-	}
-	o.podClient = clientset.CoreV1()
-
 	// Share processes
 	o.shareProcessedChanged = cmd.Flags().Changed("share-processes")
 
@@ -220,6 +211,17 @@ func (o *DebugOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st
 
 // Validate checks that the provided debug options are specified.
 func (o *DebugOptions) Validate(cmd *cobra.Command) error {
+	// CopyTo
+	// These flags are exclusive to --copy-to
+	if len(o.CopyTo) == 0 {
+		switch {
+		case o.Replace:
+			return fmt.Errorf("--replace may only be used with --copy-to.")
+		case o.SameNode:
+			return fmt.Errorf("--same-node may only be used with --copy-to.")
+		}
+	}
+
 	// Image
 	if len(o.Image) == 0 {
 		return fmt.Errorf("--image is required")
@@ -241,8 +243,8 @@ func (o *DebugOptions) Validate(cmd *cobra.Command) error {
 		return fmt.Errorf("invalid image pull policy: %s", o.PullPolicy)
 	}
 
-	// Target
+	// TargetContainer
-	if len(o.Target) > 0 && len(o.CopyTo) > 0 {
+	if len(o.TargetContainer) > 0 && len(o.CopyTo) > 0 {
 		return fmt.Errorf("--target is incompatible with --copy-to. Use --share-processes instead.")
 	}
 
@@ -258,7 +260,13 @@ func (o *DebugOptions) Validate(cmd *cobra.Command) error {
 func (o *DebugOptions) Run(f cmdutil.Factory, cmd *cobra.Command) error {
 	ctx := context.Background()
 
-	r := o.builder.
+	clientset, err := f.KubernetesClientSet()
+	if err != nil {
+		return fmt.Errorf("internal error getting clientset: %v", err)
+	}
+	o.podClient = clientset.CoreV1()
+
+	r := f.NewBuilder().
 		WithScheme(scheme.Scheme, scheme.Scheme.PrioritizedVersionsAllGroups()...).
 		NamespaceParam(o.Namespace).DefaultNamespace().ResourceNames("pods", o.TargetNames...).
 		Do()
@@ -266,7 +274,7 @@ func (o *DebugOptions) Run(f cmdutil.Factory, cmd *cobra.Command) error {
 		return err
 	}
 
-	err := r.Visit(func(info *resource.Info, err error) error {
+	err = r.Visit(func(info *resource.Info, err error) error {
 		if err != nil {
 			// TODO(verb): configurable early return
 			return err
@@ -398,7 +406,7 @@ func (o *DebugOptions) generateDebugContainer(pod *corev1.Pod) *corev1.Ephemeral
 			TerminationMessagePolicy: corev1.TerminationMessageReadFile,
 			TTY:                      o.TTY,
 		},
-		TargetContainerName: o.Target,
+		TargetContainerName: o.TargetContainer,
 	}
 
 	if o.ArgsOnly {

pkg/cmd/debug/debug_test.go

@@ -18,13 +18,18 @@ package debug
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/spf13/cobra"
 	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
+	cmdtesting "k8s.io/kubectl/pkg/cmd/testing"
 	"k8s.io/utils/pointer"
 )
 
@@ -62,10 +67,10 @@ func TestGenerateDebugContainer(t *testing.T) {
 		{
 			name: "namespace targeting",
 			opts: &DebugOptions{
-				Container:  "debugger",
+				Container:       "debugger",
-				Image:      "busybox",
+				Image:           "busybox",
-				PullPolicy: corev1.PullIfNotPresent,
+				PullPolicy:      corev1.PullIfNotPresent,
-				Target:     "myapp",
+				TargetContainer: "myapp",
 			},
 			expected: &corev1.EphemeralContainer{
 				EphemeralContainerCommon: corev1.EphemeralContainerCommon{
@@ -976,3 +981,261 @@ func TestGenerateNodeDebugPod(t *testing.T) {
 		})
 	}
 }
+
+func TestCompleteAndValidate(t *testing.T) {
+	tf := cmdtesting.NewTestFactory()
+	ioStreams, _, _, _ := genericclioptions.NewTestIOStreams()
+	cmpFilter := cmp.FilterPath(func(p cmp.Path) bool {
+		switch p.String() {
+		// IOStreams contains unexported fields
+		case "IOStreams":
+			return true
+		}
+		return false
+	}, cmp.Ignore())
+
+	tests := []struct {
+		name, args string
+		wantOpts   *DebugOptions
+		wantError  bool
+	}{
+		{
+			name:      "No targets",
+			args:      "--image=image",
+			wantError: true,
+		},
+		{
+			name:      "Invalid environment variables",
+			args:      "--image=busybox --env=FOO mypod",
+			wantError: true,
+		},
+		{
+			name:      "Invalid image name",
+			args:      "--image=image:label@deadbeef mypod",
+			wantError: true,
+		},
+		{
+			name:      "Invalid pull policy",
+			args:      "--image=image --image-pull-policy=whenever-you-feel-like-it",
+			wantError: true,
+		},
+		{
+			name:      "TTY without stdin",
+			args:      "--image=image --tty",
+			wantError: true,
+		},
+		{
+			name: "Set image pull policy",
+			args: "--image=busybox --image-pull-policy=Always mypod",
+			wantOpts: &DebugOptions{
+				Args:           []string{},
+				Image:          "busybox",
+				Namespace:      "default",
+				PullPolicy:     corev1.PullPolicy("Always"),
+				ShareProcesses: true,
+				TargetNames:    []string{"mypod"},
+			},
+		},
+		{
+			name: "Multiple targets",
+			args: "--image=busybox mypod1 mypod2",
+			wantOpts: &DebugOptions{
+				Args:           []string{},
+				Image:          "busybox",
+				Namespace:      "default",
+				ShareProcesses: true,
+				TargetNames:    []string{"mypod1", "mypod2"},
+			},
+		},
+		{
+			name: "Arguments with dash",
+			args: "--image=busybox mypod1 mypod2 -- echo 1 2",
+			wantOpts: &DebugOptions{
+				Args:           []string{"echo", "1", "2"},
+				Image:          "busybox",
+				Namespace:      "default",
+				ShareProcesses: true,
+				TargetNames:    []string{"mypod1", "mypod2"},
+			},
+		},
+		{
+			name: "Interactive no attach",
+			args: "-ti --image=busybox --attach=false mypod",
+			wantOpts: &DebugOptions{
+				Args:           []string{},
+				Attach:         false,
+				Image:          "busybox",
+				Interactive:    true,
+				Namespace:      "default",
+				ShareProcesses: true,
+				TargetNames:    []string{"mypod"},
+				TTY:            true,
+			},
+		},
+		{
+			name: "Set environment variables",
+			args: "--image=busybox --env=FOO=BAR,BAZ=BAZ mypod",
+			wantOpts: &DebugOptions{
+				Args: []string{},
+				Env: []v1.EnvVar{
+					{Name: "FOO", Value: "BAR"},
+					{Name: "BAZ", Value: "BAZ"},
+				},
+				Image:          "busybox",
+				Namespace:      "default",
+				ShareProcesses: true,
+				TargetNames:    []string{"mypod"},
+			},
+		},
+		{
+			name: "Ephemeral container: interactive session minimal args",
+			args: "mypod -it --image=busybox",
+			wantOpts: &DebugOptions{
+				Args:           []string{},
+				Attach:         true,
+				Image:          "busybox",
+				Interactive:    true,
+				Namespace:      "default",
+				ShareProcesses: true,
+				TargetNames:    []string{"mypod"},
+				TTY:            true,
+			},
+		},
+		{
+			name: "Ephemeral container: non-interactive debugger with image and name",
+			args: "--image=myproj/debug-tools --image-pull-policy=Always -c debugger mypod",
+			wantOpts: &DebugOptions{
+				Args:           []string{},
+				Container:      "debugger",
+				Image:          "myproj/debug-tools",
+				Namespace:      "default",
+				PullPolicy:     corev1.PullPolicy("Always"),
+				ShareProcesses: true,
+				TargetNames:    []string{"mypod"},
+			},
+		},
+		{
+			name:      "Ephemeral container: image not specified",
+			args:      "mypod",
+			wantError: true,
+		},
+		{
+			name:      "Ephemeral container: replace not allowed",
+			args:      "--replace --image=busybox mypod",
+			wantError: true,
+		},
+		{
+			name:      "Ephemeral container: same-node not allowed",
+			args:      "--same-node --image=busybox mypod",
+			wantError: true,
+		},
+		{
+			name: "Pod copy: interactive debug container minimal args",
+			args: "mypod -it --image=busybox --copy-to=my-debugger",
+			wantOpts: &DebugOptions{
+				Args:           []string{},
+				Attach:         true,
+				CopyTo:         "my-debugger",
+				Image:          "busybox",
+				Interactive:    true,
+				Namespace:      "default",
+				ShareProcesses: true,
+				TargetNames:    []string{"mypod"},
+				TTY:            true,
+			},
+		},
+		{
+			name: "Pod copy: non-interactive with debug container, image name and command",
+			args: "mypod --image=busybox --container=my-container --copy-to=my-debugger -- sleep 1d",
+			wantOpts: &DebugOptions{
+				Args:           []string{"sleep", "1d"},
+				Container:      "my-container",
+				CopyTo:         "my-debugger",
+				Image:          "busybox",
+				Namespace:      "default",
+				ShareProcesses: true,
+				TargetNames:    []string{"mypod"},
+			},
+		},
+		{
+			name:      "Pod copy: no image specified",
+			args:      "mypod -it --copy-to=my-debugger",
+			wantError: true,
+		},
+		{
+			name:      "Pod copy: --target not allowed",
+			args:      "mypod --target --image=busybox --copy-to=my-debugger",
+			wantError: true,
+		},
+		{
+			name: "Node: interactive session minimal args",
+			args: "node/mynode -it --image=busybox",
+			wantOpts: &DebugOptions{
+				Args:           []string{},
+				Attach:         true,
+				Image:          "busybox",
+				Interactive:    true,
+				Namespace:      "default",
+				ShareProcesses: true,
+				TargetNames:    []string{"node/mynode"},
+				TTY:            true,
+			},
+		},
+		{
+			name:      "Node: no image specified",
+			args:      "node/mynode -it",
+			wantError: true,
+		},
+		{
+			name:      "Node: replace not allowed",
+			args:      "--image=busybox --replace node/mynode",
+			wantError: true,
+		},
+		{
+			name:      "Node: same-node not allowed",
+			args:      "--image=busybox --same-node node/mynode",
+			wantError: true,
+		},
+		{
+			name:      "Node: --target not allowed",
+			args:      "node/mynode --target --image=busybox",
+			wantError: true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			opts := NewDebugOptions(ioStreams)
+			var gotError error
+
+			cmd := &cobra.Command{
+				Run: func(cmd *cobra.Command, args []string) {
+					gotError = opts.Complete(tf, cmd, args)
+					if gotError != nil {
+						return
+					}
+					gotError = opts.Validate(cmd)
+				},
+			}
+			cmd.SetArgs(strings.Split(tc.args, " "))
+			addDebugFlags(cmd, opts)
+
+			cmdError := cmd.Execute()
+
+			if tc.wantError {
+				if cmdError != nil || gotError != nil {
+					return
+				}
+				t.Fatalf("CompleteAndValidate got nil errors but wantError: %v", tc.wantError)
+			} else if cmdError != nil {
+				t.Fatalf("cmd.Execute got error '%v' executing test cobra.Command, wantError: %v", cmdError, tc.wantError)
+			} else if gotError != nil {
+				t.Fatalf("CompleteAndValidate got error: '%v', wantError: %v", gotError, tc.wantError)
+			}
+
+			if diff := cmp.Diff(tc.wantOpts, opts, cmpFilter, cmpopts.IgnoreUnexported(DebugOptions{})); diff != "" {
+				t.Error("CompleteAndValidate unexpected diff in generated object: (-want +got):\n", diff)
+			}
+		})
+	}
+}