-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
linkerd-multicluster remoteDiscoverySelector default selects all remote services #11309
Comments
same thing happening to us. if the namespace exists in both clusters, it'll mirror the service Untitled.mp4 |
Hi folks! Thank you for raising this. I attempted to reproduce and I did not manage to. I followed @willhughes-au's instructions.
My link resource:
Then annotated
Only @calvinbui thanks for the video! The content is blurred though so I can't really tell what's happening. |
the video was showing all the new services being created. as willhughes-au mentioned, if you deploy it w/o |
Ahhh @calvinbui I see, that makes sense. Based on the repro instructions I understood that simply re-linking means all services will be mirrored. I have set spec:
clusterCredentialsSecret: cluster-credentials-target
gatewayAddress: 192.168.224.2
gatewayIdentity: linkerd-gateway.linkerd-multicluster.serviceaccount.identity.linkerd.cluster.local
gatewayPort: "4143"
probeSpec:
path: /ready
period: 3s
port: "4191"
remoteDiscoverySelector: {}
selector:
matchLabels:
mirror.linkerd.io/mirror-to-clusterA: "true"
targetClusterDomain: cluster.local
targetClusterLinkerdNamespace: linkerd
targetClusterName: target
|
The multicluster link supports two selectors: one for normal (endpoint-based) mirrors, and one for remote-discovery where only service imports are created. When the remote-discovery selector is empty (i.e. an empty object '{}'), then all services in a cluster will be mirrored. The created imports also have an underlying Endpoints object created. There are two distinct checks that decide whether a service import should be created: `isExported` and `isRemote`. When a selector is empty, the checks are shortcircuted and return early. The former check (`isExported`) additionally also checks if a service is remote, without checking if the corresponding selector is empty. This allows services to slip through since an empty selector encompasses everything. The change fixes the issue by removing any remote discovery checks from `isExported`. Where necessary, we add another call to `isRemote`. Fixes #11309 Signed-off-by: Matei David <[email protected]>
This edge release introduces a fix for service discovery on endpoints that use hostPorts. Previously, the destination service would return the pod IP for the discovery request which could break connectivity on pod restart. To fix this, direct pod communication for a pod bound on a hostPort will always return the hostIP. In addition, this change fixes a security vulnerability (CVE-2023-2603) detected in the CNI plugin and proxy-init images and includes a number of other fixes and small improvements. * Addressed security vulnerability CVE-2023-2603 in proxy-init and CNI plugin ([11296]) * Introduced resource requests/limits for the policy controller resource in the control plane helm chart ([11301]) * Fixed an issue where an empty `remoteDiscoverySelector` field in a multicluster link would cause all services to be mirrored ([11309]) * Removed time out from `linkerd multicluster gateways` command; when no metrics exist the command will return instantly ([11265]) * Improved help messaging for `linkerd multicluster link` ([11265]) * Changed hostPort lookup behaviour in the destination service; previously, endpoint lookups for pods bound on a hostPort would return the Pod IP which would result in loss of connectivity on pod restart, hostIPs are now always returned when a pod uses a hostPort ([11328]) * Updated HTTPRoute webhook rule to validate all apiVersions of the resource (thanks @mikutas!) ([11149]) * Fixed erroneous `skipped` messages when injecting namespaces with `linkerd inject` (thanks @mikutas!) ([10231]) [11309]: #11309 [11296]: #11296 [11328]: #11328 [11301]: #11301 [11265]: #11265 [11149]: #11149 [10231]: #10231 Signed-off-by: Matei David <[email protected]>
* Fix mirroring all services when remote selector is empty The multicluster link supports two selectors: one for normal (endpoint-based) mirrors, and one for remote-discovery where only service imports are created. When the remote-discovery selector is empty (i.e. an empty object '{}'), then all services in a cluster will be mirrored. The created imports also have an underlying Endpoints object created. There are two distinct checks that decide whether a service import should be created: `isExported` and `isRemote`. When a selector is empty, the checks are shortcircuted and return early. The former check (`isExported`) additionally also checks if a service is remote, without checking if the corresponding selector is empty. This allows services to slip through since an empty selector encompasses everything. The change fixes the issue by removing any remote discovery checks from `isExported`. Where necessary, we add another call to `isRemote`. Fixes #11309 Signed-off-by: Matei David <[email protected]> * Add an integration test for empty selectors Signed-off-by: Matei David <[email protected]> * Differentiate test service ports Signed-off-by: Matei David <[email protected]> * @alpeb's feedback Signed-off-by: Matei David <[email protected]> --------- Signed-off-by: Matei David <[email protected]>
This edge release introduces a fix for service discovery on endpoints that use hostPorts. Previously, the destination service would return the pod IP for the discovery request which could break connectivity on pod restart. To fix this, direct pod communication for a pod bound on a hostPort will always return the hostIP. In addition, this release fixes a security vulnerability (CVE-2023-2603) detected in the CNI plugin and proxy-init images, and includes a number of other fixes and small improvements. * Addressed security vulnerability CVE-2023-2603 in proxy-init and CNI plugin ([#11296]) * Introduced resource requests/limits for the policy controller resource in the control plane helm chart ([#11301]) * Fixed an issue where an empty `remoteDiscoverySelector` field in a multicluster link would cause all services to be mirrored ([#11309]) * Removed time out from `linkerd multicluster gateways` command; when no metrics exist the command will return instantly ([#11265]) * Improved help messaging for `linkerd multicluster link` ([#11265]) * Changed how hostPort lookups are handled in the destination service. Previously, when doing service discovery for an endpoint bound on a hostPort, the destination service would return the corresponding pod IP. On pod restart, this could lead to loss of connectivity on the client's side. The destination service now always returns host IPs for service discovery on an endpoint that uses hostPorts ([#11328]) * Updated HTTPRoute webhook rule to validate all apiVersions of the resource (thanks @mikutas!) ([#11149]) * Fixed erroneous `skipped` messages when injecting namespaces with `linkerd inject` (thanks @mikutas!) ([#10231]) [#11309]: #11309 [#11296]: #11296 [#11328]: #11328 [#11301]: #11301 [#11265]: #11265 [#11149]: #11149 [#10231]: #10231 --------- Signed-off-by: Matei David <[email protected]> Co-authored-by: Eliza Weisman <[email protected]>
* Fix mirroring all services when remote selector is empty The multicluster link supports two selectors: one for normal (endpoint-based) mirrors, and one for remote-discovery where only service imports are created. When the remote-discovery selector is empty (i.e. an empty object '{}'), then all services in a cluster will be mirrored. The created imports also have an underlying Endpoints object created. There are two distinct checks that decide whether a service import should be created: `isExported` and `isRemote`. When a selector is empty, the checks are shortcircuted and return early. The former check (`isExported`) additionally also checks if a service is remote, without checking if the corresponding selector is empty. This allows services to slip through since an empty selector encompasses everything. The change fixes the issue by removing any remote discovery checks from `isExported`. Where necessary, we add another call to `isRemote`. Fixes linkerd#11309 Signed-off-by: Matei David <[email protected]> * Add an integration test for empty selectors Signed-off-by: Matei David <[email protected]> * Differentiate test service ports Signed-off-by: Matei David <[email protected]> * @alpeb's feedback Signed-off-by: Matei David <[email protected]> --------- Signed-off-by: Matei David <[email protected]>
This edge release introduces a fix for service discovery on endpoints that use hostPorts. Previously, the destination service would return the pod IP for the discovery request which could break connectivity on pod restart. To fix this, direct pod communication for a pod bound on a hostPort will always return the hostIP. In addition, this release fixes a security vulnerability (CVE-2023-2603) detected in the CNI plugin and proxy-init images, and includes a number of other fixes and small improvements. * Addressed security vulnerability CVE-2023-2603 in proxy-init and CNI plugin ([linkerd#11296]) * Introduced resource requests/limits for the policy controller resource in the control plane helm chart ([linkerd#11301]) * Fixed an issue where an empty `remoteDiscoverySelector` field in a multicluster link would cause all services to be mirrored ([linkerd#11309]) * Removed time out from `linkerd multicluster gateways` command; when no metrics exist the command will return instantly ([linkerd#11265]) * Improved help messaging for `linkerd multicluster link` ([linkerd#11265]) * Changed how hostPort lookups are handled in the destination service. Previously, when doing service discovery for an endpoint bound on a hostPort, the destination service would return the corresponding pod IP. On pod restart, this could lead to loss of connectivity on the client's side. The destination service now always returns host IPs for service discovery on an endpoint that uses hostPorts ([linkerd#11328]) * Updated HTTPRoute webhook rule to validate all apiVersions of the resource (thanks @mikutas!) ([linkerd#11149]) * Fixed erroneous `skipped` messages when injecting namespaces with `linkerd inject` (thanks @mikutas!) ([linkerd#10231]) [linkerd#11309]: linkerd#11309 [linkerd#11296]: linkerd#11296 [linkerd#11328]: linkerd#11328 [linkerd#11301]: linkerd#11301 [linkerd#11265]: linkerd#11265 [linkerd#11149]: linkerd#11149 [linkerd#10231]: linkerd#10231 --------- Signed-off-by: Matei David <[email protected]> Co-authored-by: Eliza Weisman <[email protected]>
* Fix mirroring all services when remote selector is empty The multicluster link supports two selectors: one for normal (endpoint-based) mirrors, and one for remote-discovery where only service imports are created. When the remote-discovery selector is empty (i.e. an empty object '{}'), then all services in a cluster will be mirrored. The created imports also have an underlying Endpoints object created. There are two distinct checks that decide whether a service import should be created: `isExported` and `isRemote`. When a selector is empty, the checks are shortcircuted and return early. The former check (`isExported`) additionally also checks if a service is remote, without checking if the corresponding selector is empty. This allows services to slip through since an empty selector encompasses everything. The change fixes the issue by removing any remote discovery checks from `isExported`. Where necessary, we add another call to `isRemote`. Fixes linkerd#11309 Signed-off-by: Matei David <[email protected]> * Add an integration test for empty selectors Signed-off-by: Matei David <[email protected]> * Differentiate test service ports Signed-off-by: Matei David <[email protected]> * @alpeb's feedback Signed-off-by: Matei David <[email protected]> --------- Signed-off-by: Matei David <[email protected]> Signed-off-by: Adam Shaw <[email protected]>
This edge release introduces a fix for service discovery on endpoints that use hostPorts. Previously, the destination service would return the pod IP for the discovery request which could break connectivity on pod restart. To fix this, direct pod communication for a pod bound on a hostPort will always return the hostIP. In addition, this release fixes a security vulnerability (CVE-2023-2603) detected in the CNI plugin and proxy-init images, and includes a number of other fixes and small improvements. * Addressed security vulnerability CVE-2023-2603 in proxy-init and CNI plugin ([linkerd#11296]) * Introduced resource requests/limits for the policy controller resource in the control plane helm chart ([linkerd#11301]) * Fixed an issue where an empty `remoteDiscoverySelector` field in a multicluster link would cause all services to be mirrored ([linkerd#11309]) * Removed time out from `linkerd multicluster gateways` command; when no metrics exist the command will return instantly ([linkerd#11265]) * Improved help messaging for `linkerd multicluster link` ([linkerd#11265]) * Changed how hostPort lookups are handled in the destination service. Previously, when doing service discovery for an endpoint bound on a hostPort, the destination service would return the corresponding pod IP. On pod restart, this could lead to loss of connectivity on the client's side. The destination service now always returns host IPs for service discovery on an endpoint that uses hostPorts ([linkerd#11328]) * Updated HTTPRoute webhook rule to validate all apiVersions of the resource (thanks @mikutas!) ([linkerd#11149]) * Fixed erroneous `skipped` messages when injecting namespaces with `linkerd inject` (thanks @mikutas!) ([linkerd#10231]) [linkerd#11309]: linkerd#11309 [linkerd#11296]: linkerd#11296 [linkerd#11328]: linkerd#11328 [linkerd#11301]: linkerd#11301 [linkerd#11265]: linkerd#11265 [linkerd#11149]: linkerd#11149 [linkerd#10231]: linkerd#10231 --------- Signed-off-by: Matei David <[email protected]> Co-authored-by: Eliza Weisman <[email protected]> Signed-off-by: Adam Shaw <[email protected]>
This stable release introduces a fix for service discovery on endpoints that use hostPorts. Previously, the destination service would return the pod IP associated with the endpoint which could break connectivity on pod restarts. Discovery responses have been changed to instead return the host IP. This release also fixes an issue in the multicluster extension where an empty `remoteDiscoverySelector` field in the `Link` resource would cause all services to be exported. Finally, this release addresses two security vulnerabilities, [CVE-2023-2603] and [RUSTSEC-2023-0052] respectively, and includes numerous other fixes and enhancements. * CLI * Fixed `linkerd check --proxy` incorrectly checking the proxy version of pods in the `completed` state (thanks @mikutas!) ([#11295]; fixes [#11280]) * Fixed erroneous `skipped` messages when injecting namespaces with `linkerd inject` (thanks @mikutas!) ([#10231]) * CNI * Addressed security vulnerability [CVE-2023-2603] in proxy-init and CNI plugin ([#11296]) * Control Plane * Changed how hostPort lookups are handled in the destination service. Previously, when doing service discovery for an endpoint bound on a hostPort, the destination service would return the corresponding pod IP. On pod restart, this could lead to loss of connectivity on the client's side. The destination service now always returns host IPs for service discovery on an endpoint that uses hostPorts ([#11328]) * Updated HTTPRoute webhook rule to validate all apiVersions of the resource (thanks @mikutas!) ([#11149]) * Helm * Removed unnecessary `linkerd.io/helm-release-version` annotation from the `linkerd-control-plane` Helm chart (thanks @mikutas!) ([#11329]; fixes [#10778]) * Introduced resource requests/limits for the policy controller resource in the control plane helm chart ([#11301]) * Multicluster * Fixed an issue where an empty `remoteDiscoverySelector` field in a multicluster link would cause all services to be mirrored ([#11309]) * Removed time out from `linkerd multicluster gateways` command; when no metrics exist the command will return instantly ([#11265]) * Improved help messaging for `linkerd multicluster link` ([#11265]) * Proxy * Addressed security vulnerability [RUSTSEC-2023-0052] in the proxy ([#11361]) [CVE-2023-2603]: GHSA-wp54-pwvg-rqq5 [RUSTSEC-2023-0052]: https://rustsec.org/advisories/RUSTSEC-2023-0052.html [#11295]: #11295 [#11280]: #11280 [#11361]: #11361 [#11329]: #11329 [#10778]: #10778 [#11309]: #11309 [#11296]: #11296 [#11328]: #11328 [#11301]: #11301 [#11265]: #11265 [#11149]: #11149 [#10231]: #10231 Signed-off-by: Matei David <[email protected]>
* stable-2.14.1 This stable release introduces a fix for service discovery on endpoints that use hostPorts. Previously, the destination service would return the pod IP associated with the endpoint which could break connectivity on pod restarts. Discovery responses have been changed to instead return the host IP. This release also fixes an issue in the multicluster extension where an empty `remoteDiscoverySelector` field in the `Link` resource would cause all services to be exported. Finally, this release addresses two security vulnerabilities, [CVE-2023-2603] and [RUSTSEC-2023-0052] respectively, and includes numerous other fixes and enhancements. * CLI * Fixed `linkerd check --proxy` incorrectly checking the proxy version of pods in the `completed` state (thanks @mikutas!) ([#11295]; fixes [#11280]) * Fixed erroneous `skipped` messages when injecting namespaces with `linkerd inject` (thanks @mikutas!) ([#10231]) * CNI * Addressed security vulnerability [CVE-2023-2603] in proxy-init and CNI plugin ([#11296]) * Control Plane * Changed how hostPort lookups are handled in the destination service. Previously, when doing service discovery for an endpoint bound on a hostPort, the destination service would return the corresponding pod IP. On pod restart, this could lead to loss of connectivity on the client's side. The destination service now always returns host IPs for service discovery on an endpoint that uses hostPorts ([#11328]) * Updated HTTPRoute webhook rule to validate all apiVersions of the resource (thanks @mikutas!) ([#11149]) * Helm * Removed unnecessary `linkerd.io/helm-release-version` annotation from the `linkerd-control-plane` Helm chart (thanks @mikutas!) ([#11329]; fixes [#10778]) * Introduced resource requests/limits for the policy controller resource in the control plane helm chart ([#11301]) * Multicluster * Fixed an issue where an empty `remoteDiscoverySelector` field in a multicluster link would cause all services to be mirrored ([#11309]) * Removed time out from `linkerd multicluster gateways` command; when no metrics exist the command will return instantly ([#11265]) * Improved help messaging for `linkerd multicluster link` ([#11265]) * Proxy * Addressed security vulnerability [RUSTSEC-2023-0052] in the proxy ([#11361]) [CVE-2023-2603]: GHSA-wp54-pwvg-rqq5 [RUSTSEC-2023-0052]: https://rustsec.org/advisories/RUSTSEC-2023-0052.html [#11295]: #11295 [#11280]: #11280 [#11361]: #11361 [#11329]: #11329 [#10778]: #10778 [#11309]: #11309 [#11296]: #11296 [#11328]: #11328 [#11301]: #11301 [#11265]: #11265 [#11149]: #11149 [#10231]: #10231 Signed-off-by: Matei David <[email protected]> Signed-off-by: Eliza Weisman <[email protected]> Co-authored-by: Eliza Weisman <[email protected]>
What is the issue?
remoteDiscoverySelector
was introduced on the link-crd for linkerd-multicluster in stable-2.14.Unfortunately the default behavior is that it selects all remote services.
How can it be reproduced?
Setup two K8S clusters, clusterA and clusterB with Linkerd 2.13.6 installed and working with linkerd-multicluster.
On clusterB have at least two services. Add a label on one service with
'mirror.linkerd.io/mirror-to-clusterA'=true
On clusterA add a Link resource that looks like:
Make sure the link is all working, and that the service from clusterB appears on clusterA.
Now upgrade clusterA to 2.14, including linkerd-multicluster.
Recreate the link.
Observe that all the services from clusterB are mirrored to clusterA.
To stop this behavior, you have to set the
remoteDiscoverySelector
to something that is not null or an empty set.eg:
Logs, error output, etc
I'm not sure what relevant logs there are.
The pod
linkerd-service-mirror-clusterB
on clusterA is filled with a whole lot of events about creating a new service mirror foroutput of
linkerd check -o short
Environment
Possible solution
If
remoteDiscoverySelector
is not set on the Link resource, remote discovery should not be used.Additional context
No response
Would you like to work on fixing this bug?
no
The text was updated successfully, but these errors were encountered: