Support for non-http backends

[mortenab]

Hi,

We are exposing gRPC services through Traefik with the h2c protocol on the backends. Because of the logic in getValidInstances shown below, service instances that do not have an http endpoint are considered invalid:

traefik-extra-service-fabric/servicefabric.go

Lines 202 to 215 in 6e90a9e

	func getValidInstances(sfClient sfClient, app sf.ApplicationItem, service sf.ServiceItem, partition sf.PartitionItem) []sf.InstanceItem {
	var validInstances []sf.InstanceItem
	if instances, err := sfClient.GetInstances(app.ID, service.ID, partition.PartitionInformation.ID); err != nil {
	log.Error(err)
	} else {
	for _, instance := range instances.Items {
	if isHealthy(instance.ReplicaItemBase) && hasHTTPEndpoint(instance.ReplicaItemBase) {
	validInstances = append(validInstances, instance)
	}
	}
	}
	return validInstances
	}

As a consequence, the gRPC service instances are not included when the configuration template is evaluated.

We have managed to work around this issue by first adding an additional "dummy" http endpoint to the services and then writing a custom configuration template that selects the "h2c" endpoint using the getNamedEndpoint template function.

Our workaround seems a bit fragile and it would be useful with a first order support for custom protocols - for example by using the traefik.protocol label on the service.

[lawrencegripper]

Hi,

Thanks for raising the issue, I think supporting this use case would be great.

Currently the endpoint that is exposed out by SF to our code is a combination of the uriScheme field set in the manifest (it's an attribute on the endpoint object), hostname and port. The aim of the current logic was to avoid accepting any protocols which traefik didn't support.

Would the proposal be that traefik.protocol label would be used and if this matched the uriScheme scheme specified in the SF manifest the endpoint would be added to the back-ends?

To achieve this I think we'd probably have to make two changes:

1. In the existing getValidInstances code remove the existing HTTP check (perhaps replacing it with a list of known valid protocols Traefik can serve).
2. In the template add funcs to check if the endpoint published by SF matches the traefik.protocol label set.

It feels odd having to specify both the uriScheme on the SF endpoint and add a traefik.protocol label with the same data. I think my preferred option would just be to do update the hasHTTPEndpoint func to be hasValidProtocol and have this validate the endpoint exposed by SF is routable by Traefik.

What do you think?

[jjcollinge]

Agree, I think the best approach is to have a func hasRoutableEndpoint that validates the service's protocol matches a supported list of Traefik protocols (http://, https://, h2c:// and check for other HTTP2 variants). I've called it ...Endpoint as it will allow us to extend functionality later to validate other parts of the URI schema should we need to. I think we should handle this without the need for an additional label.

[mortenab]

Hi,

Thanks for looking into this! Sounds like a good solution - I suggested the traefik.protocol label as it seems to be preferred by other providers.

As far as I can tell, Service Fabric does not support specifying UriSchemes other than tcp, http and https so the custom scheme won't actually be specified anywhere in the service manifest. We have worked around this limitation by creating our own ICommunicationListener that returns a URL on the form h2c://<server>:<port> in the OpenAsync() method. This is the endpoint URL that the Traefik provider ends up receiving.

[lawrencegripper]

Ah didn't realize the schema limited UriScheme that much. Do you think it would be better to use traefik.protocol then to remove the requirement for use to define your own ICommunicationListener?

Thinking out loud the plugin code could strip *:// off the front of the endpoint and replace it with the content of the traefik.protocol label. If one isn't provided defaulting to http

[jjcollinge]

I know having to override SF URI behaviour isn't ideal - however, that allows us to keep a single source of truth. Introducing a new label would be a potential point of conflict between our provider and the cluster configuration. Implementing an ICommunicationListen to return a custom endpoint is a fairly common pattern in SF and as long as we point to docs on how to do it I think it should be ok to do this without a new label. Happy to discuss further, this isn't a particular pain point I have to live with so @mortenab your views are key.

[mortenab]

I actually don't mind overriding the URI in the ICommunicationListener as we would have implemented the ICommunicationListener anyway. The main issues for me are:

1. We need to add a dummy http endpoint
2. We are required to override the built-in template
3. Inside the template we need to hard-code the name of the endpoint returned by the ICommunicationListener.

As far as I can see, the solution proposed by @jjcollinge will solve all three issues 👍

[lawrencegripper]

I agree, sorry for complicating matters. @mortenab are you happy to submit a PR to add this functionality?

[mortenab]

Yes - I can submit a PR. I do not have the bandwidth the coming weeks, but I will put it on the backlog.