api icon indicating copy to clipboard operation
api copied to clipboard

[WIP] Introducing alpha API for ServiceExportPolicy

Open nmittler opened this issue 4 years ago • 11 comments

This attempts to provide finer-grained control over how a services are exported across the mesh.

Allows setting both mesh-wide and namespace defaults as well as overriding defaults with policies for individual services.

nmittler avatar Feb 02 '21 19:02 nmittler

@nmittler: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
release-notes_api fc69645e7ba2fdb45625d680131ec5920bc7f551 link /test release-notes_api

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

istio-testing avatar Feb 02 '21 19:02 istio-testing

FYI @jeremyOT

nmittler avatar Feb 02 '21 20:02 nmittler

My initial gut reaction is that this feels too complicated. I was imagining something a bit simpler, just controlling generation of ServiceExport resources. Basically just a service selector to indicate which services in a namespace should be exported, and control over whether they are exported or not. Then if that is in the system namespace it would set default for the cluster.

message ServiceExportPolicy {
  ServiceSelector selector = 1;
  bool auto_export = 2;
}

The simple match-current-istio behavior would be no selector, auto_export true, placed in the mesh system namespace.

smawson avatar Feb 02 '21 21:02 smawson

My initial gut reaction is that this feels too complicated.

@smawson I don't entirely disagree. There were a few things that I was trying to fit into this, however:

  1. The ability to invert the Istio default, making service export explicit in order to match the behavior of MCS.
  2. Some way to integrate cluster-local into all of this, since it's effectively the same thing ... killing 2 birds with one stone.
  3. Unblock the MCS integration work

The only way to do 2 AFIAK is with workload selectors on both the service endpoints and client workloads.

The way I chose to do 1 was to allow ServiceExportPolicy to be defined in the istio-system namespace to change the global configuration and it seemed natural to allow a per-namespace and per-service override. I suppose we could put the global default in mesh config, but it seems a bit awkward to have 2 different APIs. A middle-ground might be to just embed the CR in mesh config, but I'm not sure if that's an improvement or not.

nmittler avatar Feb 02 '21 23:02 nmittler

@nmittler is there a RFP for this API change and how does this interact with networking.istio.io/exportTo annotation that can be added on any Kubernetes service?

nrjpoddar avatar Feb 03 '21 10:02 nrjpoddar

We should discuss (2), I don't see why it requires the workload selectors on to and from. We don't have that today for cluster-local right? With MCS model cluster-local is the default and services need to be explicitly supported, so I think we're OK with just a boolean on the mesh level, namespace level and service level, with inheritance.

smawson avatar Feb 03 '21 19:02 smawson

One thing we are looking into for the service-apis implementation is how to handle service/config import/export. Its possible we would want to define a separate policy in Istio or the k8s api, which seems plausible to have significant overlap with this. I could see us defining a policy that deals with exporting to other namespaces and exporting to other clusters.

This discussion is still pretty early though, but we may want to sync up on this when we understand the problem more (should be soon)

howardjohn avatar Feb 03 '21 22:02 howardjohn

@smawson

We should discuss (2), I don't see why it requires the workload selectors on to and from. We don't have that today for cluster-local right? With MCS model cluster-local is the default and services need to be explicitly supported, so I think we're OK with just a boolean on the mesh level, namespace level and service level, with inheritance.

I should have also added this as a goal:

  1. Support the existing exportTo capabilities for exporting to selected namespaces.

A boolean is enough to capture cluster-local vs global, but it's not sufficient to cover exporting to selected namespaces or a subset of services. Selecting a destination host (with wildcards) gets you the ability to select the services and namespaces to export to. But then you start to run up against the question "what does export really mean"? If we limit exporting to cluster-local vs mesh-wide, allowing namespace/service selection starts to break this down quickly. I believe the general solution to all of this is exporting selected service endpoints to selected workloads. This allows us fine-grained control of service exporting across the mesh, and frees us from having to think about cluster entirely (cluster is just a label we can select on). It also has implications WRT potentially simplifying some of the service exposure-related issues with mesh federation.

nmittler avatar Feb 03 '21 23:02 nmittler

@howardjohn

One thing we are looking into for the service-apis implementation is how to handle service/config import/export. Its possible we would want to define a separate policy in Istio or the k8s api, which seems plausible to have significant overlap with this. I could see us defining a policy that deals with exporting to other namespaces and exporting to other clusters.

Yeah sounds very similar. Let's sync when you're at a good point.

nmittler avatar Feb 03 '21 23:02 nmittler

Hey @nmittler - I am going to echo some of the points raised by others earlier. I would like to see a former proposal on this to introduce the problems we are trying to solve on top of what we already have (exportTo & sidecar API) and also understand how this relates to the k8s MCS API.

linsun avatar Feb 04 '21 19:02 linsun

@smawson

We should discuss (2), I don't see why it requires the workload selectors on to and from. We don't have that today for cluster-local right? With MCS model cluster-local is the default and services need to be explicitly supported, so I think we're OK with just a boolean on the mesh level, namespace level and service level, with inheritance.

I should have also added this as a goal:

  1. Support the existing exportTo capabilities for exporting to selected namespaces.

A boolean is enough to capture cluster-local vs global, but it's not sufficient to cover exporting to selected namespaces or a subset of services. Selecting a destination host (with wildcards) gets you the ability to select the services and namespaces to export to. But then you start to run up against the question "what does export really mean"? If we limit exporting to cluster-local vs mesh-wide, allowing namespace/service selection starts to break this down quickly. I believe the general solution to all of this is exporting selected service endpoints to selected workloads. This allows us fine-grained control of service exporting across the mesh, and frees us from having to think about cluster entirely (cluster is just a label we can select on). It also has implications WRT potentially simplifying some of the service exposure-related issues with mesh federation.

I don't know if I agree that supporting existing exportTo capabilities should be goal. To me this should be focused solely on controlling ServiceExport resources and nothing more. This controls whether a set of endpoints for a service are exported to the mesh-wide service or not.

Otherwise if we try to bring in the exportTo stuff it will get really complicated really quickly. What if different clusters say different things about exportTo? Do those only count in that cluster? Or do they affect all consumers of that service?

You're bringing in new requirements that make solving this correctly much harder, because the number of cases explodes. If we limit ourselves instead to just the question of when to create a ServiceExport and when not to, the design is much simpler and straightforward. It also gives us a nice composable piece we can build things like exportTo on top of.

smawson avatar Feb 04 '21 20:02 smawson

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2021-02-04. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.

Created by the issue and PR lifecycle manager.

istio-policy-bot avatar May 15 '24 23:05 istio-policy-bot