eclipse-hono / hono

Eclipse Hono™ Project
https://eclipse.dev/hono
Eclipse Public License 2.0
449 stars 137 forks source link

Optional custom mapper #1790

Closed BobClaerhout closed 4 years ago

BobClaerhout commented 4 years ago

After the discussion in the F2F last week, I hereby present you a brief overview of the discussed issue. Hopefully, I haven't got anything wrong or misinterpreted. Feel free to comment and complete where necessary.

Problem

Currenlty, Hono is payload agnostic. It does not care what payload is sent and just transparantly passes it to the business application.
However, sometimes the payload contains information which might be useful in some cases. When the payload contains the deviceId, it is useful to do a (partial) mapping in Hono of this payload in order to authorize the gateway-device combination. If this is possible, you can take advantage of the via property in Hono and authorize the gateway-device combination.
If (partial) mapping of the payload becomes possible, it might make sense to simplify the lora and sigfox adapters again (for simplification I'll only write down lora adapter but in all cases, this should relate to the sigfox adapter as well). Right now, the only difference between the lora-adapter and the http-adapter is the lora specific mapping of the metadata. If an optional hook would be introduced in hono, the lora adapter could be removed and merged with the http adapter. The optional hook for mapping metadata and payload can be specified for lora, sigfox or a custom implementation.

Proposal

Proposal

BobClaerhout commented 4 years ago

@sophokles73, @ctron WDYT? Any remarks on the design? Is this enough for you and can we start implementing it?

ctron commented 4 years ago

I am not sure about the configuration, mostly because I don't understand what you have in mind.

  • Configure mapper in adapter (via config map in all adapters) to include:
    • Mapper name
    • Mapper endpoint (HTTP or AMQP)

I would expect the tenant/gateway/device to have some kind of "mapper" endpoint configured. If it does have that, all traffic would be run through that mapper before forwarding it to the downstream message endpoint.

If we do that "by name", then we might get into trouble when it comes to multi-tenancy. As a tenant might not have access to the protocol adapter configurations. And thus could not create a new endpoint "by name". So at least, starting from the tenant level, it should be "by endpoint/URL".

Unless you want to define some system wide "well known" endpoints.

BobClaerhout commented 4 years ago

Ok, I agree this is not optimal if we have this configured in the adapter since they(some tenants in a multi-tenant installation) indeed have no access to this configuration. I think we can have them "by name" and have reserved ones. FMPOV it is best to have combination of both:

Next to that, we can allow each tenant to have it's custom mapper endpoints configured in the tenant configuration which does not have the same name as the reserved ones mentioned above (should we configure the reserved ones in the device registry for this as well?).

Nevertheless I would do this in different steps. I would first go for the system wide endpoints and later on add the ability to configure it on a tenant basis.

ctron commented 4 years ago

Nevertheless I would do this in different steps. I would first go for the system wide endpoints and later on add the ability to configure it on a tenant basis.

I am not sure that this is really necessary. If we would only allow to configure and endpoint (e.g. HTTP), then it wouldn't really matter where the value comes from (adapter, tenant, gateway), the flow would always be the same:

BobClaerhout commented 4 years ago

I'm not quite following I think. I agree the flow is in all cases the same. If we allow it to be configured on all the different levels at the same time, this just adds more complexity while I would do the additional configuration in step 2.

sophokles73 commented 4 years ago

FMPOV we should start with making it configurable at the tenant level. That will be the most useful option for almost all cases. If you run Hono for a single tenant only, then you will only need to configure a single tenant, i.e. the amount of config to do is the same as if you would configure it at the adapter level.

BobClaerhout commented 4 years ago

But what would you do for the lorawan and the sigfox mappers? I think we can remove the specific adapters and have an out of the box mapper for this. If you do this configuration at tenant level, this means you will need to configure this mapper for all tenants. If you think that's the way to go, it's ok for me but it does sound a bit odd to me to have such a configuration needed for all tenants while this can be configured in the adapter in this case.

sophokles73 commented 4 years ago

good point, and the LoRa and Sigfox adapters are the adapters that you need it for in the first place, right? I agree that it makes sense to configure this at the adapter level then as a first step. In fact, it will probably make sense to start adding it to the LoRa adapter in a first iteration and then see if and how to generalize. WDYT?

BobClaerhout commented 4 years ago

The lorawan server knows the device from which the payload is coming. Our own gateways are currently not parsing any data and do not know it. They payload however contains this data. So we need these mappers for getting the deviceId out of the payload which is sent over mqtt. However, the first iteration focusing on creating a mapper in the lora adapter is perfectly fine for me. Now that I think about it, if we merge the lora adapter again with the http adapter, we might use the used endpoint/url to figure out the mapper if needed. By doing that you do not need to configure each device but can do it by using a specific endpoint. But that's only 1 extra way to "configure" the used mapper for a device. We can discuss this later on.

sophokles73 commented 4 years ago

@BobClaerhout can we close this issue? IMHO the first iteration has been completed and we can later create new issues for adopting this functionality in other protocol adapters. WDYT?

BobClaerhout commented 4 years ago

yes, I agree. We can close the issue

sophokles73 commented 4 years ago

@BobClaerhout then be my guest ... you opened it, you close it :-)

BobClaerhout commented 4 years ago

Ok, sorry. didn't know I was supposed to :)