Entity registration API v2 proposal

[albinsuresh]

Proposed changes

• HTTP APIs for entity registration, de-registration and querying

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
• Documentation Update (if none of the other choices apply)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

Checklist

• I have read the CONTRIBUTING doc
• I have signed the CLA (in all commits with git commit -s)
• I ran cargo fmt as mentioned in CODING_GUIDELINES
• I used cargo clippy as mentioned in CODING_GUIDELINES
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

Further comments

[reubenmiller]

Since the {topic-id} includes forward slashes, this may cause some implementation problems (though it depends on the path rules on your HTTP handler from whatever library will be used). So it might be worth checking if that is possible first.

The alternative would be to force users to url encode the {topic-id}, e.g. %2F but that would be annoying to use.

[albinsuresh]

Yeah, I was expecting URL encoded values only. Isn't that widely used in query parameters and hence an accepted convention?

Even if we have to resort to non-URL encoded values, we have done something similar for our file-transfer service to capture arbitrary-length file paths. So, I'm hoping it should be okay. @jarhodes-sag could you comment on the feasibility, as you've got the most experience with HTTP APIs in Rust.

[jarhodes314]

URL encoding would be a bit annoying to use, although I think this is a relatively simple case where .replace("/", "%2F") would generally suffice. I'm fairly sure allowing people to use non-escaped entity IDs would be technically possible, but that would prevent us from doing anything of the form /v1/entity/{topic-id}/something-else (or at least I would have reservations about doing that, even if it might be technically feasible).

[reubenmiller]

The pain of having to enforce url encoding of the {topic-id} could be reduced via a tedge cli command, tedge register device device/child01//...then it can do the encoding

[reubenmiller]

Doing any url encoding (in the path) is super painful if you have to do it in bash (taken from: https://stackoverflow.com/questions/296536/how-to-urlencode-data-for-curl-command)

rawurlencode() {
  local string="${1}"
  local strlen=${#string}
  local encoded=""
  local pos c o

  for (( pos=0 ; pos<strlen ; pos++ )); do
     c=${string:$pos:1}
     case "$c" in
        [-_.~a-zA-Z0-9] ) o="${c}" ;;
        * )               printf -v o '%%%02x' "'$c"
     esac
     encoded+="${o}"
  done
  echo "${encoded}"    # You can either set a return variable (FASTER) 
  REPLY="${encoded}"   #+or echo the result (EASIER)... or both... :p
}

[reubenmiller]

Or a slightly better one (taken from the same link as above):

ENCODED_TOPIC_ID=$(curl -s -o /dev/null -w %{url_effective} --get --data-urlencode "device/main//" localhost:1883 | cut -d'?' -f2-)

[albinsuresh]

If URL encoding is not a preferred option, then we can reconsider using entity id directly in the path.

I'm fairly sure allowing people to use non-escaped entity IDs would be technically possible

It should be possible as we're doing the same in the file transfer service where we allow users to POST and GET file names of arbitrary length

but that would prevent us from doing anything of the form /v1/entity/{topic-id}/something-else

Although I can't think of any API requirements that would add further path segments after the entity id, at least for the current requirements, I don't want to rule out future evolutions of the API that would require it. But, even in that case we have a way around it, as the entity topic IDs are fixed length (at most 4 path segments). So, even if we introduce such an API in future, we can take advantage of this fixed length. The only niggle there would be to use empty path segments for things like device/main/// which may not be a commonly accepted convention.

I wasn't planning to enforce 4 segment entity IDs for the proposed APIs, and let users just provide just device/main instead of device/main/// in the path, and the server logic to append any missing trailing /s to compute the entity topic id, to keep it simple. We can enforce the 4-segment length only when a future API enhancement absolutely requires it.

[albinsuresh]

Although I can't think of any API requirements that would add further path segments after the entity id

That statement didn't age well 😉. Already came across this requirement for bulk deletion of child entities of a given entity topic-id.

[didier-wenzek]

Although I can't think of any API requirements that would add further path segments after the entity id

That statement didn't age well 😉. Already came across this requirement for bulk deletion of child entities of a given entity topic-id.

I see things the other way around. Since its more convenient to have the entity topic identifier used as a suffix (to re-conciliate MQTT and HTTP naming rues), we can take this as a starting point for URL design, e.g. /v1/entity/{topic-id} This is also aligned with the REST principles where an URL identifies an entity and not some action.

[albinsuresh]

This is slightly unconventional, to DELETE a resource using query parameters. The conventional way would have been to use sub paths of the target resource:

DELETE /v1/entities/{topic-id}/children

But, I chose to avoid sub-paths due to the empty / characters in the topic-id as follows:

DELETE /v1/entities/device/child01///children

...which is not an acceptable convention either.

Another alternative would have been to provide a dedicated path with the target entity topic-id provided as a query parameter as follows:

DELETE /v1/entities/children?%40topic-id=device/child01//

But, I don't find it to be much of an improvement from the existing version either.

[didier-wenzek]

Do we really need a way to DELETE in one operation all the child-devices of given device?

Isn't a child-devices query and a series of one-by-one DELETEs enough?

[reubenmiller]

Do we really need a way to DELETE in one operation all the child-devices of given device?

Isn't a child-devices query and a series of one-by-one DELETEs enough?

I would agree with @didier-wenzek. We don't need an explicit "delete all children" API command as we can do a query and then do individual api calls (we could also wrap the calls in a special tedge cli command to still do the same function but not require a specific API call).

[albinsuresh]

Addressed by eb0eba8

[didier-wenzek]

I'm aligned with what is proposed here:

• Move the responsibility of the entity store to the agent (to ensure consistency independently of the cloud mappers).
• Use HTTP instead of MQTT to update the entity store (to return status and error along a request/response protocol)
• Use MQTT to notify all the components of any entity registration update.

I vote against bulk updates and deletes. There is also no need for pagination.This API should focus on the content and its primary clients are scripts not users.

[didier-wenzek]

Approved

[didier-wenzek]

Suggested change

	* Status: __New__
	* Status: __Approved__

[github-actions]

Robot Results

✅ Passed	❌ Failed	⏭️ Skipped	Total	Pass %	⏱️ Duration
520	0	2	520	100	1h40m44.692156999s