vtctld: guard ev.ShardInfo before overwriting response keyspace/shard in reparent RPCs

PlannedReparentShard and EmergencyReparentShard both initialize the
response with req.Keyspace/req.Shard, then overwrite from ev.ShardInfo
inside an `if ev != nil` block. ev itself is always non-nil (allocated
before the reparent call), but ev.ShardInfo is zero-valued whenever
reparentShardLocked returns before populating it — for example when no
valid reparent target exists in the same cell as the current primary.

Calling .Keyspace() / .ShardName() on a zero-valued topo.ShardInfo
dereferences an internal nil pointer, crashing vtctld.

Fix: check whether ev.ShardInfo.Keyspace() is non-empty before
overwriting the response fields. When it is empty the response already
holds the correct values from the req initialization, so the fallback
is safe.

Signed-off-by: Lucas Morduchowicz <lmorduch@gmail.com>

Author: lmorduch

go/vt/vtctl/grpcvtctldserver/server.go

@@ -1332,9 +1332,14 @@ func (s *VtctldServer) EmergencyReparentShard(ctx context.Context, req *vtctldat
 		Shard:    req.Shard,
 	}
 
+	// ev is always non-nil (initialized before the reparent call), but ev.ShardInfo may be
+	// zero-valued if reparentShardLocked returned before populating it (e.g. error paths where
+	// no primary could be found). Guard against a nil pointer dereference.
 	if ev != nil {
-		resp.Keyspace = ev.ShardInfo.Keyspace()
-		resp.Shard = ev.ShardInfo.ShardName()
+		if k := ev.ShardInfo.Keyspace(); k != "" {
+			resp.Keyspace = k
+			resp.Shard = ev.ShardInfo.ShardName()
+		}
 
 		if ev.NewPrimary != nil && !topoproto.TabletAliasIsZero(ev.NewPrimary.Alias) {
 			resp.PromotedPrimary = ev.NewPrimary.Alias
@@ -3333,9 +3338,14 @@ func (s *VtctldServer) PlannedReparentShard(ctx context.Context, req *vtctldatap
 		Shard:    req.Shard,
 	}
 
+	// ev is always non-nil (initialized before the reparent call), but ev.ShardInfo may be
+	// zero-valued if reparentShardLocked returned before populating it (e.g. error paths where
+	// no primary could be found). Guard against a nil pointer dereference.
 	if ev != nil {
-		resp.Keyspace = ev.ShardInfo.Keyspace()
-		resp.Shard = ev.ShardInfo.ShardName()
+		if k := ev.ShardInfo.Keyspace(); k != "" {
+			resp.Keyspace = k
+			resp.Shard = ev.ShardInfo.ShardName()
+		}
 
 		if ev.NewPrimary != nil && !topoproto.TabletAliasIsZero(ev.NewPrimary.Alias) {
 			resp.PromotedPrimary = ev.NewPrimary.Alias

go/vt/vtctl/grpcvtctldserver/server_test.go

@@ -9011,6 +9011,46 @@ func TestPlannedReparentShard(t *testing.T) {
 			expectEventsToOccur: true,
 			expectedErr:         "global status vars failed",
 		},
+		{
+			// Regression test: when reparentShardLocked returns an error before populating
+			// ev.ShardInfo (e.g. the ExpectedPrimary check fires before ev.ShardInfo is
+			// assigned), the response must still carry the keyspace/shard from the request
+			// rather than empty strings.
+			name: "response preserves keyspace/shard when reparent fails before ShardInfo is populated",
+			ts:   memorytopo.NewServer(ctx, "zone1"),
+			tablets: []*topodatapb.Tablet{
+				{
+					Alias: &topodatapb.TabletAlias{
+						Cell: "zone1",
+						Uid:  100,
+					},
+					Type: topodatapb.TabletType_PRIMARY,
+					PrimaryTermStartTime: &vttime.Time{
+						Seconds: 100,
+					},
+					Keyspace: "testkeyspace",
+					Shard:    "-",
+				},
+			},
+			tmc: &testutil.TabletManagerClient{},
+			req: &vtctldatapb.PlannedReparentShardRequest{
+				Keyspace: "testkeyspace",
+				Shard:    "-",
+				// ExpectedPrimary deliberately points to a non-existent tablet so that
+				// reparentShardLocked fails the expected-primary check before assigning
+				// ev.ShardInfo, leaving ev.ShardInfo zero-valued.
+				ExpectedPrimary: &topodatapb.TabletAlias{
+					Cell: "zone1",
+					Uid:  999,
+				},
+			},
+			expected: &vtctldatapb.PlannedReparentShardResponse{
+				Keyspace: "testkeyspace",
+				Shard:    "-",
+			},
+			expectEventsToOccur: false,
+			expectedErr:         "primary zone1-0000000100 is not equal to expected alias zone1-0000000999",
+		},
 	}
 
 	for _, tt := range tests {
@@ -9041,6 +9081,9 @@ func TestPlannedReparentShard(t *testing.T) {
 
 			if tt.expectedErr != "" {
 				assert.EqualError(t, err, tt.expectedErr)
+				if tt.expected != nil {
+					testutil.AssertPlannedReparentShardResponsesEqual(t, tt.expected, resp)
+				}
 
 				return
 			}