[WIP] Introducing alpha API for ServiceExportPolicy #1870

nmittler · 2021-02-02T19:06:58Z

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.

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.

istio-testing · 2021-02-02T19:37:50Z

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

Test name	Commit	Details	Rerun command
release-notes_api	`fc69645`	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.

nmittler · 2021-02-02T20:15:17Z

FYI @JeremyOT

smawson · 2021-02-02T21:20:28Z

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.

frankbu · 2021-02-02T22:11:23Z

networking/v1alpha3/service_export_policy.proto

+//
+// ServiceExportPolicy supports a variety of use cases that require partitioning the mesh
+// and providing customized views of service endpoints to each workload. For example,
+// it may be desirable that workloads in a given cluster use only the service endpoints


Isn't this already handled quite simply in Istio? When/why would/should one use ServiceExportPolicy vs. locality LB, to restrict calls to the same cluster?

I think I agree with @smawson's comment, this seems like it might be overkill. Do we really need all the flexibility?

Also, I assume we would want to use ServiceExportPolicy to control the generating of MCS ServiceExports. If that's correct, it seems like the "to" part can't be honoured, since MCS doesn't support exporting to a subset of the workloads, or even subset of clusters.

Isn't this already handled quite simply in Istio? When/why would/should one use ServiceExportPolicy vs. locality LB, to restrict calls to the same cluster?

Locality LB can only restrict calls to region, zone, subzone. It doesn't address cluster at all. The only API for cluster-local today is in mesh config, which isn't really accessible for service owners.

Also, I assume we would want to use ServiceExportPolicy to control the generating of MCS ServiceExports. If that's correct, it seems like the "to" part can't be honoured, since MCS doesn't support exporting to a subset of the workloads, or even subset of clusters.

I was more focused on thinking about what we think the Istio API should be, rather than being strictly restricted to the current set of capabilities afforded by the MCS API. If we decide we'd like more control, we could help drive additional requirements for MCS.

nmittler · 2021-02-02T23:24:53Z

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:

The ability to invert the Istio default, making service export explicit in order to match the behavior of MCS.
Some way to integrate cluster-local into all of this, since it's effectively the same thing ... killing 2 birds with one stone.
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.

nrjpoddar · 2021-02-03T10:06:43Z

@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?

smawson · 2021-02-03T19:17:20Z

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.

howardjohn · 2021-02-03T22:43:25Z

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)

nmittler · 2021-02-03T23:01:46Z

@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:

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 · 2021-02-03T23:03:49Z

@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.

linsun · 2021-02-04T19:07:50Z

networking/v1alpha3/service_export_policy.proto

+// it may be desirable that workloads in a given cluster use only the service endpoints
+// residing within the same cluster.
+//
+// By default (if no ServiceExportPolicy is specified), all services are exported to all


would prefer to remove by default... it is possible vendor may choose to have a service export policy in place as the default.

linsun · 2021-02-04T19:14:39Z

networking/v1alpha3/service_export_policy.proto

+// workloads in the mesh. This means that any workload can call any service endpoint.
+//
+// The mesh-wide default can be overridden by applying a ServiceExportPolicy to the
+// Istio system namespace, using the service wildcard. For example, the following will


or a root namespace

linsun · 2021-02-04T19:20:07Z

networking/v1alpha3/service_export_policy.proto

+// spec:
+//   from:
+//     service: * # Using wildcard applies to all services.
+//   # No "to" is specified, so services will not export by default.


I think this example is somewhat confusing. what if I remove this? Does it also mean everything is not exported?

// from: // service: * # Using wildcard applies to all services.

Do you think it would be useful to have a config in the API to indicate it is globally auto exported vs not? like our mtls strict or none api...

linsun · 2021-02-04T19:40:44Z

networking/v1alpha3/service_export_policy.proto

+//   name: auto-export-off
+//   namespace: example-ns
+// spec:
+//   from:


I think the from and to in the export service policy is very confusing. I had envisioned this to be service export policy at a cluster level. Within the cluster, we already have sidecar API where you could specify ingress and egress. How is this different?

linsun · 2021-02-04T19:44:32Z

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.

frankbu · 2021-02-04T20:49:07Z

networking/v1alpha3/service_export_policy.proto

+// kind: ServiceExportPolicy
+// metadata:
+//   name: myservice-cluster1-to-cluster1-cluster2
+//   namespace: example-ns
+// spec:
+//   from:
+//     service: MyService
+//     selector:
+//       "topology.istio.io/cluster": cluster1
+//   to:
+//     hosts:
+//     - *
+//     selector:
+//       "topology.istio.io/cluster": cluster1


This seems like a strange example. Isn't this just configuring what would be the default if there was no export at all?

smawson · 2021-02-04T20:52:36Z

@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:

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.

istio-policy-bot · 2024-05-15T23:32:37Z

🚧 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.

nmittler requested review from costinm, howardjohn, josephpeacock and smawson February 2, 2021 19:06

nmittler requested review from dcberg, duderino, linsun, louiscryan and nrjpoddar as code owners February 2, 2021 19:06

google-cla bot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Feb 2, 2021

istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 2, 2021

nmittler changed the title ~~Introducing alpha API for ServiceExportPolicy~~ [WIP] Introducing alpha API for ServiceExportPolicy Feb 2, 2021

istio-testing added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Feb 2, 2021

nmittler force-pushed the export-policy branch 2 times, most recently from 36670d8 to ad885a7 Compare February 2, 2021 19:30

Introducing alpha API for ServiceExportPolicy

fc69645

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 force-pushed the export-policy branch from ad885a7 to fc69645 Compare February 2, 2021 19:37

nmittler mentioned this pull request Feb 2, 2021

Adding a global configuration field to control MCS serviceexport creation behavior #1785

Closed

frankbu reviewed Feb 2, 2021

View reviewed changes

linsun reviewed Feb 4, 2021

View reviewed changes

frankbu reviewed Feb 4, 2021

View reviewed changes

howardjohn removed the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label May 15, 2024

istio-policy-bot closed this May 15, 2024

istio-policy-bot added the lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. label May 15, 2024

[WIP] Introducing alpha API for ServiceExportPolicy #1870

[WIP] Introducing alpha API for ServiceExportPolicy #1870

Uh oh!

Conversation

nmittler commented Feb 2, 2021

Uh oh!

istio-testing commented Feb 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nmittler commented Feb 2, 2021

Uh oh!

smawson commented Feb 2, 2021

Uh oh!

frankbu Feb 2, 2021

Choose a reason for hiding this comment

Uh oh!

nmittler Feb 2, 2021

Choose a reason for hiding this comment

Uh oh!

nmittler commented Feb 2, 2021

Uh oh!

nrjpoddar commented Feb 3, 2021

Uh oh!

smawson commented Feb 3, 2021

Uh oh!

howardjohn commented Feb 3, 2021

Uh oh!

nmittler commented Feb 3, 2021

Uh oh!

nmittler commented Feb 3, 2021

Uh oh!

linsun Feb 4, 2021

Choose a reason for hiding this comment

Uh oh!

linsun Feb 4, 2021

Choose a reason for hiding this comment

Uh oh!

linsun Feb 4, 2021

Choose a reason for hiding this comment

Uh oh!

linsun Feb 4, 2021

Choose a reason for hiding this comment

Uh oh!

linsun commented Feb 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

frankbu Feb 4, 2021

Choose a reason for hiding this comment

Uh oh!

smawson commented Feb 4, 2021

Uh oh!

istio-policy-bot commented May 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

istio-testing commented Feb 2, 2021 •

edited

Loading

linsun commented Feb 4, 2021 •

edited

Loading