PR #14057 follow up fix: service id parsing from sidecar id (#14541)

* fix service id parsing from sidecar id

* simplify suffix trimming

agent/agent.go

@@ -2612,7 +2612,7 @@ func (a *Agent) removeServiceLocked(serviceID structs.ServiceID, persist bool) e
 }
 
 func (a *Agent) removeServiceSidecars(serviceID structs.ServiceID, persist bool) error {
-	sidecarSID := structs.NewServiceID(sidecarServiceID(serviceID.ID), &serviceID.EnterpriseMeta)
+	sidecarSID := structs.NewServiceID(sidecarIDFromServiceID(serviceID.ID), &serviceID.EnterpriseMeta)
 	if sidecar := a.State.Service(sidecarSID); sidecar != nil {
 		// Double check that it's not just an ID collision and we actually added
 		// this from a sidecar.

agent/sidecar_service.go

@@ -10,12 +10,15 @@ import (
 	"github.com/hashicorp/consul/agent/structs"
 )
 
-func sidecarServiceID(serviceID string) string {
-	return serviceID + "-sidecar-proxy"
+const sidecarIDSuffix = "-sidecar-proxy"
+
+func sidecarIDFromServiceID(serviceID string) string {
+	return serviceID + sidecarIDSuffix
 }
 
-func serviceIDFromSidecarID(sidecarServiceID string) string {
-	return strings.Split(sidecarServiceID, "-")[0]
+// reverses the sidecarIDFromServiceID operation
+func serviceIDFromSidecarID(sidecarID string) string {
+	return strings.TrimSuffix(sidecarID, sidecarIDSuffix)
 }
 
 // sidecarServiceFromNodeService returns a *structs.NodeService representing a
@@ -48,7 +51,7 @@ func sidecarServiceFromNodeService(ns *structs.NodeService, token string) (*stru
 
 	// Override the ID which must always be consistent for a given outer service
 	// ID. We rely on this for lifecycle management of the nested definition.
-	sidecar.ID = sidecarServiceID(ns.ID)
+	sidecar.ID = sidecarIDFromServiceID(ns.ID)
 
 	// Set some meta we can use to disambiguate between service instances we added
 	// later and are responsible for deregistering.

agent/sidecar_service_test.go

@@ -325,12 +325,14 @@ func TestAgent_SidecarPortFromServiceID(t *testing.T) {
 func TestAgent_SidecarDefaultChecks(t *testing.T) {
 	tests := []struct {
 		name                 string
+		serviceID            string
 		svcAddress           string
 		proxyLocalSvcAddress string
 		port                 int
 		wantChecks           []*structs.CheckType
 	}{{
 		name:                 "uses proxy address for check",
+		serviceID:            "web1-1-sidecar-proxy",
 		svcAddress:           "123.123.123.123",
 		proxyLocalSvcAddress: "255.255.255.255",
 		port:                 2222,
@@ -341,13 +343,14 @@ func TestAgent_SidecarDefaultChecks(t *testing.T) {
 				Interval: 10 * time.Second,
 			},
 			{
-				Name:         "Connect Sidecar Aliasing web1",
-				AliasService: "web1",
+				Name:         "Connect Sidecar Aliasing web1-1",
+				AliasService: "web1-1",
 			},
 		},
 	},
 		{
 			name:                 "uses proxy.local_service_address for check if proxy address is empty",
+			serviceID:            "web1-1-sidecar-proxy",
 			proxyLocalSvcAddress: "1.2.3.4",
 			port:                 2222,
 			wantChecks: []*structs.CheckType{
@@ -357,15 +360,33 @@ func TestAgent_SidecarDefaultChecks(t *testing.T) {
 					Interval: 10 * time.Second,
 				},
 				{
-					Name:         "Connect Sidecar Aliasing web1",
-					AliasService: "web1",
+					Name:         "Connect Sidecar Aliasing web1-1",
+					AliasService: "web1-1",
+				},
+			},
+		},
+		{
+			name:                 "redundant name",
+			serviceID:            "1-sidecar-proxy-web1-sidecar-proxy",
+			svcAddress:           "123.123.123.123",
+			proxyLocalSvcAddress: "255.255.255.255",
+			port:                 2222,
+			wantChecks: []*structs.CheckType{
+				{
+					Name:     "Connect Sidecar Listening",
+					TCP:      "123.123.123.123:2222",
+					Interval: 10 * time.Second,
+				},
+				{
+					Name:         "Connect Sidecar Aliasing 1-sidecar-proxy-web1",
+					AliasService: "1-sidecar-proxy-web1",
 				},
 			},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			gotChecks := sidecarDefaultChecks("web1", tt.svcAddress, tt.proxyLocalSvcAddress, tt.port)
+			gotChecks := sidecarDefaultChecks(tt.serviceID, tt.svcAddress, tt.proxyLocalSvcAddress, tt.port)
 			require.Equal(t, tt.wantChecks, gotChecks)
 		})
 	}