Repository: linkerd/linkerd2
Pull request #7243 (2021-11-09 01:57:31)

Fix identity overrides for endpoint slices
When the `mirror.linkerd.io/remote-gateway-identity` and `mirror.linkerd.io/remote-svc-fq-name` annotations are set on an EndpointSlice object, the destination controller does not return the correct identity hints for that endpoint.

We fix an incorrect assignment to fix this.  We also fix some logic that can result in a nil pointer dereference instead of comparing empty strings.  We add a test case to exercise these.

Signed-off-by: Alex Leong <alex@buoyant.io>

<!--  Thanks for sending a pull request!

If you already have a well-structured git commit message, chances are GitHub
set the title and description of this PR to the git commit message subject and
body, respectively. If so, you may delete these instructions and submit your PR.

If this is your first time, please read our contributor guide:
https://github.com/linkerd/linkerd2/blob/main/CONTRIBUTING.md

The title and description of your Pull Request should match the git commit
subject and body, respectively. Git commit messages are structured as follows:

```
Subject

Problem

Solution

Validation

Fixes #[GitHub issue ID]

DCO Sign off
```

Example git commit message:

```
Introduce Pull Request Template

GitHub's community guidelines recommend a pull request template, the repo was
lacking one.

Introduce a `PULL_REQUEST_TEMPLATE.md` file.

Once merged, the
[Community profile checklist](https://github.com/linkerd/linkerd2/community)
should indicate the repo now provides a pull request template.

Fixes #3321

Signed-off-by: Jane Smith <jane.smith@example.com>
```

Note the git commit message subject becomes the pull request title.

For more details around git commits, see the section on Committing in our
contributor guide:
https://github.com/linkerd/linkerd2/blob/main/CONTRIBUTING.md#committing
-->



Patch:
diff --git a/controller/api/destination/watcher/endpoints_watcher.go b/controller/api/destination/watcher/endpoints_watcher.go
index de686b0cf..7c6cfe4fe 100644
--- a/controller/api/destination/watcher/endpoints_watcher.go
+++ b/controller/api/destination/watcher/endpoints_watcher.go
@@ -712,7 +712,7 @@ func (pp *portPublisher) endpointSliceToAddresses(es *discovery.EndpointSlice) A
 
 				identity := es.Annotations[consts.RemoteGatewayIdentity]
 				address, id := pp.newServiceRefAddress(resolvedPort, IPAddr, serviceID.Name, es.Namespace)
-				address.Identity, address.AuthorityOverride = authorityOverride, identity
+				address.Identity, address.AuthorityOverride = identity, authorityOverride
 
 				if endpoint.Hints != nil {
 					zones := make([]discovery.ForZone, len(endpoint.Hints.ForZones))
@@ -838,7 +838,11 @@ func (pp *portPublisher) resolveESTargetPort(slicePorts []discovery.EndpointPort
 		return Port(pp.targetPort.IntVal)
 	case intstr.String:
 		for _, p := range slicePorts {
-			if *p.Name == pp.targetPort.StrVal {
+			name := ""
+			if p.Name != nil {
+				name = *p.Name
+			}
+			if name == pp.targetPort.StrVal {
 				return Port(*p.Port)
 			}
 		}


Test patch:
diff --git a/controller/api/destination/watcher/endpoints_watcher_test.go b/controller/api/destination/watcher/endpoints_watcher_test.go
index 992bef815..4dc7453f5 100644
--- a/controller/api/destination/watcher/endpoints_watcher_test.go
+++ b/controller/api/destination/watcher/endpoints_watcher_test.go
@@ -1544,6 +1544,7 @@ func TestEndpointsWatcherServiceMirrors(t *testing.T) {
 		expectedAddresses                []string
 		expectedNoEndpoints              bool
 		expectedNoEndpointsServiceExists bool
+		enableEndpointSlices             bool
 	}{
 		{
 			k8sConfigs: []string{`
@@ -1586,6 +1587,45 @@ subsets:
 			k8sConfigs: []string{`
 apiVersion: v1
 kind: Service
+metadata:
+  name: name1-remote
+  namespace: ns
+spec:
+  type: LoadBalancer
+  ports:
+  - port: 8989`,
+				`
+apiVersion: discovery.k8s.io/v1beta1
+kind: EndpointSlice
+metadata:
+  name: name1-remote-xxxx
+  namespace: ns
+  annotations:
+    mirror.linkerd.io/remote-gateway-identity: "gateway-identity-1"
+    mirror.linkerd.io/remote-svc-fq-name: "name1-remote-fq"
+  labels:
+    mirror.linkerd.io/mirrored-service: "true"
+    kubernetes.io/service-name: name1-remote
+endpoints:
+- addresses:
+  - 172.17.0.12
+ports:
+- port: 8989`,
+			},
+			serviceType: "mirrored service with identity and endpoint slices",
+			id:          ServiceID{Name: "name1-remote", Namespace: "ns"},
+			port:        8989,
+			expectedAddresses: []string{
+				"172.17.0.12:8989/gateway-identity-1/name1-remote-fq:8989",
+			},
+			expectedNoEndpoints:              false,
+			expectedNoEndpointsServiceExists: false,
+			enableEndpointSlices:             true,
+		},
+		{
+			k8sConfigs: []string{`
+apiVersion: v1
+kind: Service
 metadata:
   name: name1-remote
   namespace: ns
@@ -1701,7 +1741,7 @@ subsets:
 				t.Fatalf("NewFakeAPI returned an error: %s", err)
 			}
 
-			watcher := NewEndpointsWatcher(k8sAPI, logging.WithField("test", t.Name()), false)
+			watcher := NewEndpointsWatcher(k8sAPI, logging.WithField("test", t.Name()), tt.enableEndpointSlices)
 
 			k8sAPI.Sync(nil)