brocaar / chirpstack-application-server

ChirpStack Application Server is an open-source LoRaWAN application-server.
https://www.chirpstack.io
MIT License
501 stars 326 forks source link

Changing service profile of application does not change devices' service profile in network database #691

Open fortender opened 2 years ago

fortender commented 2 years ago

What happened?

ChirpStack Application Server API lets you change an application's service profile. Due to the fact there's a separation of network and application specific data, the device entity in the network database has its own service profile id attribute. It seems that changing the application's service profile does not cause the underlying devices to update their service profile id in the network database. You can see that the corresponding code updates the application but not the underlying devices. In my opinion that leads to a inconsistency in which the application-side has the new service profile, while the network-side does not.

What did you expect?

Changing an application's service profile should cause all underlying devices to update their service profile id in the network database. That would require the code that is linked above to instruct the network server to update all devices inside that application.

Steps to reproduce this issue

Steps:

  1. Create two service profiles X and Y
  2. Create an application A (id: 1) with service profile X
  3. Create a device D in application A and verify via database that the device has service profile X in the network database
  4. PUT /api/applications/1
    {
    "application": {
    "name": "A",
    "serviceProfileID": "[uuid of Y]"
    }
    }
  5. Device D in network database will still have service profile X while the corresponding application has the new service profile Y
no

Your Environment

Cloned chirpstack-docker repo which currently uses the docker images with tag 3. I attempted to obtain AS and NS versions via chirpstack-application-server version command inside the container but that lead to an empty output.

brocaar commented 2 years ago

I think I would rather prevent applications from changing their service-profile. There is no guarantee that the service-profile exists within the NS database, as there could be multiple service-profiles, fragmented over multiple NS databases.

brocaar commented 2 years ago

Just checked, there is already code that validates that the service-profile exists on the NS instance.

fortender commented 2 years ago

Just checked, there is already code that validates that the service-profile exists on the NS instance.

Yes. When you update an application, a call to storage.GetServiceProfile is performed. And this method does indeed check via the NS client if the service profile exists. I guess you are also talking about this code snippet.

I think I would rather prevent applications from changing their service-profile. There is no guarantee that the service-profile exists within the NS database, as there could be multiple service-profiles, fragmented over multiple NS databases.

Now that we're sure that the existence of the service-profile on the NS instance is verified, there's no reason why we shouldn't allow an application to switch the service-profile?

We basically need to retrieve a list containing all device EUIs of the application. For each device we need to determine the NS instance (assuming an application's devices can span over multiple NS instances), create a client and send an update request with the new service-profile id. And all that in a transactional manner.

I can create a draft, if you want to.