catalog: Simplify ListAllowedEndpointsForService

[draychev]

This GitHub Issue is to simplify MeshCataloger's ListAllowedEndpointsForService implementation.

• is it possible to use sets instead of iterations over lists (we mention intersection in the comments - set operations would be more efficient)
• should ListAllowedEndpointsforService be renamed to ListEndpointsForTargetServiceIdentity or something, which:
  • implies direction from/to
  • removes Service and uses ServiceIdentity (which could map to a K8s Service Account in the context of Kubernetes)
• ListAllowedOutboundServiceIdentities could be removed from the MeshCataloger interface - it is only used within MeshCatalog:

  osm/pkg/catalog/endpoint.go

  Line 50 in 6722573

  destSvcAccounts, err := mc.ListAllowedOutboundServiceIdentities(downstreamIdentity)

• rename destSvcAccount variable to destinationServiceIdentity for correctness (this is no longer a Service Account)
• other opportunities to simplify ListAllowedEndpointsForService
  • drop Allowed from the name - this should be implied from ListEndpointsForService
  • The signature of the function is func (mc *MeshCatalog) ListAllowedEndpointsForService(downstreamIdentity identity.K8sServiceAccount, upstreamSvc service.MeshService) ([]endpoint.Endpoint, error) { -- it is unclear - are we listing endpoints for upstreamSvc from the perspective of downstreamIdentity? Or vice versa?

osm/pkg/catalog/endpoint.go

Lines 42 to 70 in 6722573

	// for the given downstream identity
	func (mc *MeshCatalog) ListAllowedEndpointsForService(downstreamIdentity identity.K8sServiceAccount, upstreamSvc service.MeshService) ([]endpoint.Endpoint, error) {
	outboundEndpoints, err := mc.listEndpointsForService(upstreamSvc)
	if err != nil {
	log.Error().Err(err).Msgf("Error looking up endpoints for upstream service %s", upstreamSvc)
	return nil, err
	}
	destSvcAccounts, err := mc.ListAllowedOutboundServiceIdentities(downstreamIdentity)
	if err != nil {
	log.Error().Err(err).Msgf("Error looking up outbound service accounts for downstream identity %s", downstreamIdentity)
	return nil, err
	}
	// allowedEndpoints comprises of only those endpoints from outboundEndpoints that matches the endpoints from listEndpointsforIdentity
	// i.e. only those interseting endpoints are taken into cosideration
	var allowedEndpoints []endpoint.Endpoint
	for _, destSvcAccount := range destSvcAccounts {
	podEndpoints := mc.listEndpointsforIdentity(destSvcAccount)
	for _, ep := range outboundEndpoints {
	for _, podIP := range podEndpoints {
	if ep.IP.Equal(podIP.IP) {
	allowedEndpoints = append(allowedEndpoints, ep)
	}
	}
	}
	}
	return allowedEndpoints, nil
	}

---

This is a sub task of #2218

[draychev]

Another question to consider - why is

func (mc *MeshCatalog) ListAllowedEndpointsForService(downstreamIdentity identity.ServiceIdentity, upstreamSvc service.MeshService) ([]endpoint.Endpoint, error) {

returning []endpoint.Endpoints? What if it returned something, which then can be used to query the ProxyRegistry, which will then return endpoints?

So

something := meshCatalog.ListAllowedEndpointsForService(downstreamIdentity, upstreamSvc service.MeshService)
endpoints := proxyRegistry.GetEndpoints(something)

I am not sure what something could be. Is that a list of EndpointIdentity or []ProxyIdentity? --> #3186 (comment)

[allenlsy]

Works done for this issue:

• Used hash map for computing the intersection of allowed outbound endpoints and endpoints for service identities. The time complexity is reduced from O(n^3) to O(n^2)
• Renamed destSvcAccount variable to destServiceIdentity
• Renamed method ListAllowedEndpointsForService to ListEndpointsForServiceIdentity
• Dropped allowed wording from method names. Mesh services assume that certain services needs authorization to be accessed.
• For the unclear method signature func (mc *MeshCatalog) ListAllowedEndpointsForService(downstreamIdentity identity.K8sServiceAccount, upstreamSvc service.MeshService), I don't have a better idea. Using downstream and upstream indicates the direction. Thus I only complemented the method comment.

These are done in PR #3626 and #3744