reuse cidr flag completion from ssh cmd for ssh-patch (#204)

* reuse cidr flag completion from ssh cmd in ssh-patch * PR feedback
gardener · Jan 12, 2023 · 47954d6 · 47954d6
1 parent 41f6bd5
commit 47954d6
Show file tree

Hide file tree

Showing 11 changed files with 164 additions and 118 deletions.
diff --git a/docs/help/gardenctl_ssh-patch.md b/docs/help/gardenctl_ssh-patch.md
@@ -20,7 +20,12 @@ gardenctl ssh-patch cli-xxxxxxxx
 
 ```
       --cidr stringArray   CIDRs to allow access to the bastion host; if not given, your system's public IPs (v4 and v6) are auto-detected.
+      --control-plane      target control plane of shoot, use together with shoot argument
+      --garden string      target the given garden cluster
   -h, --help               help for ssh-patch
+      --project string     target the given project
+      --seed string        target the given seed cluster
+      --shoot string       target the given shoot cluster
 ```
 
 ### Options inherited from parent commands

diff --git a/pkg/cmd/base/options.go b/pkg/cmd/base/options.go
@@ -20,16 +20,14 @@ import (
 
 //go:generate mockgen -destination=./mocks/mock_options.go -package=mocks github.com/gardener/gardenctl-v2/pkg/cmd/base CommandOptions
 
-// CommandOptions is the base interface for command options.
-type CommandOptions interface {
+// Runnable is the base interface for command options.
+type Runnable interface {
 	// Complete adapts from the command line args to the data required.
 	Complete(util.Factory, *cobra.Command, []string) error
 	// Validate validates the provided options.
 	Validate() error
 	// Run does the actual work of the command.
 	Run(util.Factory) error
-	// AddFlags adds flags to adjust the output to a cobra command.
-	AddFlags(*pflag.FlagSet)
 }
 
 // Options contains all settings that are used across all commands in gardenctl.
@@ -41,10 +39,10 @@ type Options struct {
 	Output string
 }
 
-var _ CommandOptions = &Options{}
+var _ Runnable = &Options{}
 
 // WrapRunE creates a cobra RunE function that has access to the factory.
-func WrapRunE(o CommandOptions, f util.Factory) func(cmd *cobra.Command, args []string) error {
+func WrapRunE(o Runnable, f util.Factory) func(cmd *cobra.Command, args []string) error {
 	return func(cmd *cobra.Command, args []string) error {
 		if err := o.Complete(f, cmd, args); err != nil {
 			return fmt.Errorf("failed to complete command options: %w", err)

diff --git a/pkg/cmd/ssh/access_config.go b/pkg/cmd/ssh/access_config.go
@@ -1,3 +1,9 @@
+/*
+SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Gardener contributors
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
 package ssh
 
 import (
@@ -9,15 +15,14 @@ import (
 	"time"
 
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 
 	"github.com/gardener/gardenctl-v2/internal/util"
-	"github.com/gardener/gardenctl-v2/pkg/cmd/base"
 )
 
-// AccessConfig is a struct used by all ssh related commands.
+// AccessConfig is a struct that is embedded in the options of ssh related commands.
 type AccessConfig struct {
-	base.Options
-
 	// CIDRs is a list of IP address ranges to be allowed for accessing the
 	// created Bastion host. If not given, gardenctl will attempt to
 	// auto-detect the user's IP and allow only it (i.e. use a /32 netmask).
@@ -28,7 +33,7 @@ type AccessConfig struct {
 	AutoDetected bool
 }
 
-func (o *AccessConfig) Complete(f util.Factory, cmd *cobra.Command, args []string) error {
+func (o *AccessConfig) Complete(f util.Factory, _ *cobra.Command, _ []string, ioStreams util.IOStreams) error {
 	if len(o.CIDRs) == 0 {
 		ctx, cancel := context.WithTimeout(f.Context(), 60*time.Second)
 		defer cancel()
@@ -48,7 +53,7 @@ func (o *AccessConfig) Complete(f util.Factory, cmd *cobra.Command, args []strin
 			name = "CIDRs"
 		}
 
-		fmt.Fprintf(o.IOStreams.Out, "Auto-detected your system's %s as %s\n", name, strings.Join(cidrs, ", "))
+		fmt.Fprintf(ioStreams.Out, "Auto-detected your system's %s as %s\n", name, strings.Join(cidrs, ", "))
 
 		o.CIDRs = cidrs
 		o.AutoDetected = true
@@ -70,3 +75,76 @@ func (o *AccessConfig) Validate() error {
 
 	return nil
 }
+
+func (o *AccessConfig) AddFlags(flags *pflag.FlagSet) {
+	flags.StringArrayVar(&o.CIDRs, "cidr", nil, "CIDRs to allow access to the bastion host; if not given, your system's public IPs (v4 and v6) are auto-detected.")
+}
+
+type (
+	cobraCompletionFunc          func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective)
+	cobraCompletionFuncWithError func(f util.Factory) ([]string, error)
+)
+
+func completionWrapper(f util.Factory, ioStreams util.IOStreams, completer cobraCompletionFuncWithError) cobraCompletionFunc {
+	return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
+		result, err := completer(f)
+		if err != nil {
+			fmt.Fprintf(ioStreams.ErrOut, "%v\n", err)
+			return nil, cobra.ShellCompDirectiveNoFileComp
+		}
+
+		return util.FilterStringsByPrefix(toComplete, result), cobra.ShellCompDirectiveNoFileComp
+	}
+}
+
+func RegisterCompletionFuncsForAccessConfigFlags(cmd *cobra.Command, factory util.Factory, ioStreams util.IOStreams, flags *pflag.FlagSet) {
+	utilruntime.Must(cmd.RegisterFlagCompletionFunc("cidr", completionWrapper(factory, ioStreams, cidrFlagCompletionFunc)))
+}
+
+func cidrFlagCompletionFunc(f util.Factory) ([]string, error) {
+	var addresses []string
+
+	ctx := f.Context()
+
+	publicIPs, err := f.PublicIPs(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, ip := range publicIPs {
+		cidr := ipToCIDR(ip)
+		addresses = append(addresses, fmt.Sprintf("%s\t<public>", cidr))
+	}
+
+	interfaces, err := net.Interfaces()
+	if err != nil {
+		return nil, err
+	}
+
+	includeFlags := net.FlagUp
+	excludeFlags := net.FlagLoopback
+
+	for _, iface := range interfaces {
+		addrs, err := iface.Addrs()
+		if err != nil {
+			return nil, err
+		}
+
+		if is(iface, includeFlags) && isNot(iface, excludeFlags) {
+			for _, addr := range addrs {
+				addressComp := fmt.Sprintf("%s\t%s", addr.String(), iface.Name)
+				addresses = append(addresses, addressComp)
+			}
+		}
+	}
+
+	return addresses, nil
+}
+
+func is(i net.Interface, flags net.Flags) bool {
+	return i.Flags&flags != 0
+}
+
+func isNot(i net.Interface, flags net.Flags) bool {
+	return i.Flags&flags == 0
+}
diff --git a/pkg/cmd/ssh/options.go b/pkg/cmd/ssh/options.go
@@ -34,6 +34,7 @@ import (
 	gutil "github.com/gardener/gardener/pkg/utils/gardener"
 	"github.com/gardener/gardener/pkg/utils/secrets"
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 	"golang.org/x/crypto/ssh"
 	"golang.org/x/crypto/ssh/agent"
 	corev1 "k8s.io/api/core/v1"
@@ -156,6 +157,7 @@ var (
 //
 //nolint:revive
 type SSHOptions struct {
+	base.Options
 	AccessConfig
 	// Interactive can be used to toggle between gardenctl just
 	// providing the bastion host while keeping it alive (non-interactive),
@@ -193,20 +195,26 @@ type SSHOptions struct {
 // NewSSHOptions returns initialized SSHOptions.
 func NewSSHOptions(ioStreams util.IOStreams) *SSHOptions {
 	return &SSHOptions{
-		AccessConfig: AccessConfig{
-			Options: base.Options{
-				IOStreams: ioStreams,
-			},
+		AccessConfig: AccessConfig{},
+		Options: base.Options{
+			IOStreams: ioStreams,
 		},
 		Interactive: true,
 		WaitTimeout: 10 * time.Minute,
 		KeepBastion: false,
 	}
 }
 
+func (o *SSHOptions) AddFlags(flagSet *pflag.FlagSet) {
+	flagSet.BoolVar(&o.Interactive, "interactive", o.Interactive, "Open an SSH connection instead of just providing the bastion host (only if NODE_NAME is provided).")
+	flagSet.StringVar(&o.SSHPublicKeyFile, "public-key-file", "", "Path to the file that contains a public SSH key. If not given, a temporary keypair will be generated.")
+	flagSet.DurationVar(&o.WaitTimeout, "wait-timeout", o.WaitTimeout, "Maximum duration to wait for the bastion to become available.")
+	flagSet.BoolVar(&o.KeepBastion, "keep-bastion", o.KeepBastion, "Do not delete immediately when gardenctl exits (Bastions will be garbage-collected after some time)")
+}
+
 // Complete adapts from the command line args to the data required.
 func (o *SSHOptions) Complete(f util.Factory, cmd *cobra.Command, args []string) error {
-	if err := o.AccessConfig.Complete(f, cmd, args); err != nil {
+	if err := o.AccessConfig.Complete(f, cmd, args, o.Options.IOStreams); err != nil {
 		return err
 	}
 
@@ -262,18 +270,16 @@ func ipToCIDR(address string) string {
 
 // Validate validates the provided SSHOptions.
 func (o *SSHOptions) Validate() error {
-	if o.WaitTimeout == 0 {
-		return errors.New("the maximum wait duration must be non-zero")
+	if err := o.Options.Validate(); err != nil {
+		return err
 	}
 
-	if len(o.CIDRs) == 0 {
-		return errors.New("must at least specify a single CIDR to allow access to the bastion")
+	if err := o.AccessConfig.Validate(); err != nil {
+		return err
 	}
 
-	for _, cidr := range o.CIDRs {
-		if _, _, err := net.ParseCIDR(cidr); err != nil {
-			return fmt.Errorf("CIDR %q is invalid: %w", cidr, err)
-		}
+	if o.WaitTimeout == 0 {
+		return errors.New("the maximum wait duration must be non-zero")
 	}
 
 	content, err := os.ReadFile(o.SSHPublicKeyFile)

diff --git a/pkg/cmd/ssh/ssh.go b/pkg/cmd/ssh/ssh.go
@@ -8,7 +8,6 @@ package ssh
 
 import (
 	"fmt"
-	"net"
 
 	"github.com/spf13/cobra"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -40,87 +39,14 @@ func NewCmdSSH(f util.Factory, o *SSHOptions) *cobra.Command {
 		RunE: base.WrapRunE(o, f),
 	}
 
-	cmd.Flags().BoolVar(&o.Interactive, "interactive", o.Interactive, "Open an SSH connection instead of just providing the bastion host (only if NODE_NAME is provided).")
-	cmd.Flags().StringArrayVar(&o.CIDRs, "cidr", nil, "CIDRs to allow access to the bastion host; if not given, your system's public IPs (v4 and v6) are auto-detected.")
-	cmd.Flags().StringVar(&o.SSHPublicKeyFile, "public-key-file", "", "Path to the file that contains a public SSH key. If not given, a temporary keypair will be generated.")
-	cmd.Flags().DurationVar(&o.WaitTimeout, "wait-timeout", o.WaitTimeout, "Maximum duration to wait for the bastion to become available.")
-	cmd.Flags().BoolVar(&o.KeepBastion, "keep-bastion", o.KeepBastion, "Do not delete immediately when gardenctl exits (Bastions will be garbage-collected after some time)")
+	o.AddFlags(cmd.Flags())
+	o.AccessConfig.AddFlags(cmd.Flags())
+	RegisterCompletionFuncsForAccessConfigFlags(cmd, f, o.IOStreams, cmd.Flags())
 
 	manager, err := f.Manager()
 	utilruntime.Must(err)
 	manager.TargetFlags().AddFlags(cmd.Flags())
 	flags.RegisterCompletionFuncsForTargetFlags(cmd, f, o.IOStreams, cmd.Flags())
 
-	registerCompletionFuncForFlags(cmd, f, o.IOStreams)
-
 	return cmd
 }
-
-func registerCompletionFuncForFlags(cmd *cobra.Command, f util.Factory, ioStreams util.IOStreams) {
-	utilruntime.Must(cmd.RegisterFlagCompletionFunc("cidr", completionWrapper(f, ioStreams, cidrFlagCompletionFunc)))
-}
-
-type (
-	cobraCompletionFunc          func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective)
-	cobraCompletionFuncWithError func(f util.Factory) ([]string, error)
-)
-
-func completionWrapper(f util.Factory, ioStreams util.IOStreams, completer cobraCompletionFuncWithError) cobraCompletionFunc {
-	return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-		result, err := completer(f)
-		if err != nil {
-			fmt.Fprintf(ioStreams.ErrOut, "%v\n", err)
-			return nil, cobra.ShellCompDirectiveNoFileComp
-		}
-
-		return util.FilterStringsByPrefix(toComplete, result), cobra.ShellCompDirectiveNoFileComp
-	}
-}
-
-func cidrFlagCompletionFunc(f util.Factory) ([]string, error) {
-	var addresses []string
-
-	ctx := f.Context()
-
-	publicIPs, err := f.PublicIPs(ctx)
-	if err != nil {
-		return nil, err
-	}
-
-	for _, ip := range publicIPs {
-		cidr := ipToCIDR(ip)
-		addresses = append(addresses, fmt.Sprintf("%s\t<public>", cidr))
-	}
-
-	interfaces, err := net.Interfaces()
-	if err != nil {
-		return nil, err
-	}
-
-	includeFlags := net.FlagUp
-	excludeFlags := net.FlagLoopback
-
-	for _, iface := range interfaces {
-		addrs, err := iface.Addrs()
-		if err != nil {
-			return nil, err
-		}
-
-		if is(iface, includeFlags) && isNot(iface, excludeFlags) {
-			for _, addr := range addrs {
-				addressComp := fmt.Sprintf("%s\t%s", addr.String(), iface.Name)
-				addresses = append(addresses, addressComp)
-			}
-		}
-	}
-
-	return addresses, nil
-}
-
-func is(i net.Interface, flags net.Flags) bool {
-	return i.Flags&flags != 0
-}
-
-func isNot(i net.Interface, flags net.Flags) bool {
-	return i.Flags&flags == 0
-}
diff --git a/pkg/cmd/sshpatch/bastionlistpatcher.go b/pkg/cmd/sshpatch/bastionlistpatcher.go
@@ -1,3 +1,9 @@
+/*
+SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Gardener contributors
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
 package sshpatch
 
 import (

diff --git a/pkg/cmd/sshpatch/completions.go b/pkg/cmd/sshpatch/completions.go
@@ -1,3 +1,9 @@
+/*
+SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Gardener contributors
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
 package sshpatch
 
 import (

diff --git a/pkg/cmd/sshpatch/export_test.go b/pkg/cmd/sshpatch/export_test.go
@@ -43,10 +43,9 @@ func NewTestOptions() *TestOptions {
 
 	return &TestOptions{
 		options: options{
-			AccessConfig: ssh.AccessConfig{
-				Options: base.Options{
-					IOStreams: streams,
-				},
+			AccessConfig: ssh.AccessConfig{},
+			Options: base.Options{
+				IOStreams: streams,
 			},
 		},
 		Out: out,