From 326401efaf6bf35b12f005b7aa6d79b6e415e786 Mon Sep 17 00:00:00 2001
From: Ted Teng <ted.teng@sap.com>
Date: Mon, 15 Jan 2024 10:06:14 +0800
Subject: [PATCH] per feedback

---
 pkg/cmd/ssh/connect_information.go | 29 +++++++++++++
 pkg/cmd/ssh/options.go             | 67 ++++--------------------------
 2 files changed, 37 insertions(+), 59 deletions(-)

diff --git a/pkg/cmd/ssh/connect_information.go b/pkg/cmd/ssh/connect_information.go
index dc19308a..594a40d9 100644
--- a/pkg/cmd/ssh/connect_information.go
+++ b/pkg/cmd/ssh/connect_information.go
@@ -35,6 +35,9 @@ type ConnectInformation struct {
 
 	// User is the name of the Shoot cluster node ssh login username
 	User string
+
+	// Machines is a list of machines name
+	Machines []string
 }
 
 var _ fmt.Stringer = &ConnectInformation{}
@@ -87,6 +90,7 @@ func NewConnectInformation(
 	sshPrivateKeyFile PrivateKeyFile,
 	nodePrivateKeyFiles []PrivateKeyFile,
 	nodes []corev1.Node,
+	machines []string,
 	user string,
 ) (*ConnectInformation, error) {
 	var nodeList []Node
@@ -144,6 +148,7 @@ func NewConnectInformation(
 		NodeHostname:        nodeHostname,
 		NodePrivateKeyFiles: nodePrivateKeyFiles,
 		Nodes:               nodeList,
+		Machines:            machines,
 		User:                user,
 	}, nil
 }
@@ -195,6 +200,30 @@ func (p *ConnectInformation) String() string {
 			return ""
 		}
 
+		if len(p.Machines) != 0 && len(p.Machines) != len(p.Nodes) {
+			type empty struct{}
+
+			nodeSets := make(map[string]empty, len(p.Nodes))
+
+			var missingNodes []string
+
+			for _, node := range p.Nodes {
+				nodeSets[node.Name] = empty{}
+			}
+
+			for _, machine := range p.Machines {
+				if _, ok := nodeSets[machine]; !ok {
+					missingNodes = append(missingNodes, machine)
+				}
+			}
+
+			for _, node := range missingNodes {
+				table.Rows = append(table.Rows, metav1.TableRow{
+					Cells: []interface{}{node, "unknown", "unknown"},
+				})
+			}
+		}
+
 		fmt.Fprintln(&buf, "")
 	}
 
diff --git a/pkg/cmd/ssh/options.go b/pkg/cmd/ssh/options.go
index ffcc6555..1ef050c1 100644
--- a/pkg/cmd/ssh/options.go
+++ b/pkg/cmd/ssh/options.go
@@ -231,11 +231,6 @@ type SSHOptions struct {
 	// ConfirmAccessRestriction, when set to true, implies the user understands the access restrictions for the targeted shoot.
 	// In this case, the access restriction banner is displayed without further confirmation.
 	ConfirmAccessRestriction bool
-
-	// Operator is determines whether the command should be have the operator role assigned.
-	Operator bool
-
-	client client.Client
 }
 
 // NewSSHOptions returns initialized SSHOptions.
@@ -252,7 +247,6 @@ func NewSSHOptions(ioStreams util.IOStreams) *SSHOptions {
 		NoKeepalive:           false,
 		BastionPort:           strconv.Itoa(SSHPort),
 		User:                  DefaultUsername,
-		Operator:              false,
 	}
 }
 
@@ -543,27 +537,6 @@ func (o *SSHOptions) Run(f util.Factory) error {
 		return err
 	}
 
-	if shoot.Status.TechnicalID == "" {
-		return fmt.Errorf("shoot technical ID is empty: %w", err)
-	}
-
-	// check operator role
-	if !o.Operator {
-		seedTarget := target.NewTarget(currentTarget.GardenName(), "", *shoot.Spec.SeedName, "")
-
-		seedClient, err := manager.SeedClient(ctx, seedTarget)
-		if err != nil {
-			if !apierrors.IsForbidden(err) {
-				return nil
-			}
-
-			return err
-		}
-
-		o.Operator = true
-		o.client = seedClient
-	}
-
 	// check access restrictions
 	ok, err := o.checkAccessRestrictions(manager.Configuration(), currentTarget.GardenName(), f.TargetFlags(), shoot)
 	if err != nil {
@@ -674,47 +647,22 @@ func (o *SSHOptions) Run(f util.Factory) error {
 
 	if !o.Interactive {
 		var nodes []corev1.Node
+
+		machines := make([]string, 0)
+
 		if nodeHostname == "" {
 			nodes, err = getNodes(ctx, shootClient)
 			if err != nil {
 				return fmt.Errorf("failed to list shoot cluster nodes: %w", err)
 			}
-		}
 
-		if o.Operator {
-			machines, err := getMachines(ctx, o.client, shoot.Status.TechnicalID)
+			machines, err = getNodeNamesFromMachines(ctx, manager, currentTarget)
 			if err != nil {
-				return fmt.Errorf("failed to list shoot cluster machines: %w", err)
-			}
-
-			if len(machines) != len(nodes) {
-				type empty struct{}
-
-				nodeSets := make(map[string]empty, len(nodes))
-
-				for _, node := range nodes {
-					nodeSets[node.Name] = empty{}
+				if !apierrors.IsForbidden(err) {
+					return nil
 				}
 
-				for _, machine := range machines {
-					if _, ok := machine.Labels[machinev1alpha1.NodeLabelKey]; !ok {
-						continue
-					}
-
-					if _, ok := nodeSets[machine.Labels[machinev1alpha1.NodeLabelKey]]; !ok {
-						nodes = append(nodes, corev1.Node{
-							ObjectMeta: metav1.ObjectMeta{Name: machine.Labels[machinev1alpha1.NodeLabelKey]},
-							Status: corev1.NodeStatus{
-								Conditions: []corev1.NodeCondition{
-									{
-										Type:   corev1.NodeReady,
-										Status: corev1.ConditionUnknown,
-									},
-								},
-							},
-						})
-					}
-				}
+				return fmt.Errorf("failed to get shoot cluster node names from machines: %w", err)
 			}
 		}
 
@@ -728,6 +676,7 @@ func (o *SSHOptions) Run(f util.Factory) error {
 			o.SSHPrivateKeyFile,
 			nodePrivateKeyFiles,
 			nodes,
+			machines,
 			o.User,
 		)
 		if err != nil {