Skip to content

ForeignServices cardinality issues #305

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

Closed
louiscryan opened this issue Jan 11, 2018 · 5 comments
Closed

ForeignServices cardinality issues #305

louiscryan opened this issue Jan 11, 2018 · 5 comments
Assignees

Comments

@louiscryan
Copy link
Contributor

@frankbu @wora @rshriram

Currently ForegnServices has the following properties

  • There can only be one resource of this type in the whole API
  • It defines a plural

The alternatives would be to (a) allow there to be many and (b) allow one per domain

@rshriram
Copy link
Member

Thanks for following up on this.. Parroting back what I told @frankbu

The idea is that you have only one such thing that requires admin access to add. Per @nmnellis (Weather.com), this seems like a nice thing.

The more technical problem is that we no longer do anything with namespaces in the rules. Even in routing rules, we infer the namespace for short names based on the calling pod's namespace, instead of the rule's namespace. The rationale behind this was that most people tend to put all Istio artifacts in the istio-system namespace, for easier management.

From a cross platform support perspective, this alienation of namespaces works well, because namespace is just a folder where you put your rules. It is not an isolation/protection/enforcement domain.

So, if you look at foreign services from this perspective, its simpler to do any RBAC if need be, based on the fact that there is only one foreign services resource to track. Having per namespace means that the underlying system assigns the endpoints to unique namespaces - a property that only kubernetes has. Any bespoke system using internal service registry on a bunch of containers/vms/baremetals or using Consul or using CloudFoundry will not have this abstraction.

The same argument can be applied to RouteRules and DestinationRules as well. It remains to be sorted out.

cc @ZackButcher / @ijsnellf / @GregHanson as well, since you would probably have encountered this.

@frankbu
Copy link
Contributor

frankbu commented Jan 12, 2018

A couple of thoughts.

  1. Does this need to be one or the other? If the resource contains a list of multiple foreign services, but it was not specified that there can only be one, then users would have the choice to put all the foreign services in a single resource (@nmnellis's preference), one foreign resource per resource (extreme opposite), or anything in between. The compete list of foreign services is the union of services from all the resources (no namespaces involved).

  2. Regardless of whether there is one or many of the resources allowed, I think the name is bad. My suggestion is to rename ForeignServices to something like EgressSpecification (or something like that - not plural) and rename the embedded Service to ForeignService, since that's what it actually is.

@rshriram
Copy link
Member

#307 fixes the cardinality issue. I am not sold on the term EgressRule, as there is no action associated with that rule. EgressSpecification is fine, but if the current PR #307 suffices, then all should be good.

@geeknoid
Copy link
Contributor

Is this still relevant? Can we close the issue?

incfly pushed a commit to incfly/api that referenced this issue Jun 13, 2018
* Handle RequestStatus error

* Add test for RequestStatus error
@andraxylia
Copy link
Contributor

ServiceEntry fixes these problems.

nacx pushed a commit to nacx/api that referenced this issue Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants