ssh: Add flag to disable keepalive (#249)

* fix logs * Add --no-keepalive flag * make gen-markdown * add tests for no-keepalive
gardener · Mar 20, 2023 · 5fd2dd9 · 5fd2dd9
1 parent 0877bcd
commit 5fd2dd9
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 10 deletions.
diff --git a/docs/help/gardenctl_ssh.md b/docs/help/gardenctl_ssh.md
@@ -15,6 +15,7 @@ gardenctl ssh [NODE_NAME] [flags]
   -h, --help                      help for ssh
       --interactive               Open an SSH connection instead of just providing the bastion host (only if NODE_NAME is provided). (default true)
       --keep-bastion              Do not delete immediately when gardenctl exits (Bastions will be garbage-collected after some time)
+      --no-keepalive              Exit after the bastion host became available without keeping the bastion alive or establishing an SSH connection. Note that this flag requires the flags --interactive=false and --keep-bastion to be set
       --project string            target the given project
       --public-key-file string    Path to the file that contains a public SSH key. If not given, a temporary keypair will be generated.
       --seed string               target the given seed cluster

diff --git a/pkg/cmd/ssh/options.go b/pkg/cmd/ssh/options.go
@@ -151,6 +151,7 @@ var (
 
 		return cmd.Run()
 	}
+
 	// waitForSignal informs the user about their SSHOptions and keeps the
 	// bastion alive until gardenctl exits.
 	waitForSignal = func(ctx context.Context, o *SSHOptions, shootClient client.Client, bastion *operationsv1alpha1.Bastion, nodeHostname string, nodePrivateKeyFiles []string, signalChan <-chan struct{}) error {
@@ -291,6 +292,12 @@ type SSHOptions struct {
 	// SkipAvailabilityCheck determines whether to check for the availability of
 	// the bastion host.
 	SkipAvailabilityCheck bool
+
+	// NoKeepalive controls if the command should exit after the bastion becomes available.
+	// If this option is true, no SSH connection will be established and the bastion will
+	// not be kept alive after it became available.
+	// This option can only be used if KeepBastion is set to true and Interactive is set to false.
+	NoKeepalive bool
 }
 
 // NewSSHOptions returns initialized SSHOptions.
@@ -304,6 +311,7 @@ func NewSSHOptions(ioStreams util.IOStreams) *SSHOptions {
 		WaitTimeout:           10 * time.Minute,
 		KeepBastion:           false,
 		SkipAvailabilityCheck: false,
+		NoKeepalive:           false,
 	}
 }
 
@@ -313,6 +321,7 @@ func (o *SSHOptions) AddFlags(flagSet *pflag.FlagSet) {
 	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)")
 	flagSet.BoolVar(&o.SkipAvailabilityCheck, "skip-availability-check", o.SkipAvailabilityCheck, "Skip checking for SSH bastion host availability.")
+	flagSet.BoolVar(&o.NoKeepalive, "no-keepalive", o.NoKeepalive, "Exit after the bastion host became available without keeping the bastion alive or establishing an SSH connection. Note that this flag requires the flags --interactive=false and --keep-bastion to be set")
 }
 
 // Complete adapts from the command line args to the data required.
@@ -385,6 +394,16 @@ func (o *SSHOptions) Validate() error {
 		return errors.New("the maximum wait duration must be non-zero")
 	}
 
+	if o.NoKeepalive {
+		if o.Interactive {
+			return errors.New("set --interactive=false when disabling keepalive")
+		}
+
+		if !o.KeepBastion {
+			return errors.New("set --keep-bastion when disabling keepalive")
+		}
+	}
+
 	content, err := os.ReadFile(o.SSHPublicKeyFile)
 	if err != nil {
 		return fmt.Errorf("invalid SSH public key file: %w", err)
@@ -672,15 +691,15 @@ func (o *SSHOptions) Run(f util.Factory) error {
 
 	logger.Info("Bastion host became available.", "address", printAddr)
 
-	if nodeHostname != "" && o.Interactive {
-		err = remoteShell(ctx, o, bastion, nodeHostname, nodePrivateKeyFiles)
-	} else {
-		err = waitForSignal(ctx, o, shootClient, bastion, nodeHostname, nodePrivateKeyFiles, ctx.Done())
+	if o.NoKeepalive {
+		return nil
 	}
 
-	logger.V(4).Info("Exiting…")
+	if nodeHostname == "" || !o.Interactive {
+		return waitForSignal(ctx, o, shootClient, bastion, nodeHostname, nodePrivateKeyFiles, ctx.Done())
+	}
 
-	return err
+	return remoteShell(ctx, o, bastion, nodeHostname, nodePrivateKeyFiles)
 }
 
 func (o *SSHOptions) bastionIngressPolicies(logger klog.Logger, providerType string) ([]operationsv1alpha1.BastionIngressPolicy, error) {
@@ -698,7 +717,7 @@ func (o *SSHOptions) bastionIngressPolicies(logger klog.Logger, providerType str
 					return nil, fmt.Errorf("GCP only supports IPv4: %s", cidr)
 				}
 
-				logger.Info("GCP only supports IPv4, skipped CIDR: %s\n", cidr)
+				logger.Info("GCP only supports IPv4, skipped CIDR: %s\n", "cidr", cidr)
 
 				continue // skip
 			}
@@ -760,7 +779,7 @@ func cleanup(ctx context.Context, o *SSHOptions, gardenClient client.Client, bas
 			}
 		}
 	} else {
-		logger.Info("Keeping bastion", klog.KObj(bastion))
+		logger.Info("Keeping bastion", "bastion", klog.KObj(bastion))
 
 		if o.generatedSSHKeys {
 			logger.Info("The SSH keypair for the bastion remain on disk", "publicKeyPath", o.SSHPublicKeyFile, "privateKeyPath", o.SSHPrivateKeyFile)
@@ -855,7 +874,7 @@ func waitForBastion(ctx context.Context, o *SSHOptions, gardenClient client.Clie
 		}
 
 		if o.SkipAvailabilityCheck {
-			fmt.Fprintln(o.IOStreams.Out, "Bastion is ready, skipping availability check")
+			logger.Info("Bastion is ready, skipping availability check")
 			return true, nil
 		}
 

diff --git a/pkg/cmd/ssh/ssh_test.go b/pkg/cmd/ssh/ssh_test.go
@@ -523,7 +523,34 @@ var _ = Describe("SSH Command", func() {
 
 			Expect(cmd.RunE(cmd, nil)).To(Succeed())
 
-			Expect(out.String()).To(ContainSubstring("Bastion is ready, skipping availability check"))
+			Expect(logs).To(ContainSubstring("Bastion is ready, skipping availability check"))
+		})
+
+		It("should not keep alive the bastion", func() {
+			options := ssh.NewSSHOptions(streams)
+			options.NoKeepalive = true
+			options.KeepBastion = true
+			options.Interactive = false
+
+			cmd := ssh.NewCmdSSH(factory, options)
+
+			ssh.SetWaitForSignal(func(ctx context.Context, o *ssh.SSHOptions, shootClient client.Client, bastion *operationsv1alpha1.Bastion, nodeHostname string, nodePrivateKeyFiles []string, signalChan <-chan struct{}) error {
+				err := errors.New("this function should not be executed as of NoKeepalive = true")
+				Fail(err.Error())
+				return err
+			})
+			ssh.SetExecCommand(func(ctx context.Context, command string, args []string, o *ssh.SSHOptions) error {
+				err := errors.New("this function should not be executed as of NoKeepalive = true")
+				Fail(err.Error())
+				return err
+			})
+
+			// simulate an external controller processing the bastion and proving a successful status
+			go waitForBastionThenSetBastionReady(ctx, gardenClient, bastionName, *testProject.Spec.Namespace, bastionHostname, bastionIP)
+
+			Expect(cmd.RunE(cmd, nil)).To(Succeed())
+
+			Expect(logs).To(ContainSubstring("Bastion host became available."))
 		})
 	})
 
@@ -598,6 +625,28 @@ var _ = Describe("SSH Options", func() {
 		Expect(o.Validate()).NotTo(Succeed())
 	})
 
+	Context("no-keepalive", func() {
+		It("should require non-interactive mode", func() {
+			o := ssh.NewSSHOptions(streams)
+			o.NoKeepalive = true
+			o.KeepBastion = true
+
+			o.Interactive = true
+
+			Expect(o.Validate()).NotTo(Succeed())
+		})
+
+		It("should require keep bastion", func() {
+			o := ssh.NewSSHOptions(streams)
+			o.NoKeepalive = true
+			o.Interactive = false
+
+			o.KeepBastion = false
+
+			Expect(o.Validate()).NotTo(Succeed())
+		})
+	})
+
 	It("should require a public SSH key file", func() {
 		o := ssh.NewSSHOptions(streams)
 		o.CIDRs = []string{"8.8.8.8/32"}