id request field completely removed from deploymentBase feedback DDI API

[diegorondini]

Hi,

I think there's been an unintended change in DDI APIs: the id request field is no longer one of the parameters of deploymentBase feedback DDI API.

The bad thing is that the id request field was in practice mandatory until 0.3M7 (included):

According to the official DDI API hawkbit documentation, the feedbackID was never mandatory as a part of the request body because it was already given by the actionID path variable. But the current implementation was not working without it and resulted in a HTTP 404 error code.
#1091

Even though the intention was to deprecate it:
bacd72f
https://github.com/eclipse/hawkbit/releases/tag/0.3.0 (see section "Improvements")
that deprecation has never made it to a release as it's been removed before 0.3M8 release:
3271867#diff-608ea86c7c080d1c5742fa8001855af723d34ceb6aadb234a49367d5bfc52c4aL39

So the result is that:

1. deploymentBase feedback requests including the id succeed with hawkBit ≤ 0.3M7, but fail with ≥ 0.3M8
2. deploymentBase feedback requests NOT including the id fail with hawkBit ≤ 0.3M7, but succeed with ≥ 0.3M8

For API stability I think the id should be just ignored when provided.

[avgustinmm]

@diegorondini ,
it's already a pretty old change, and this is the first complaint regarding it. Is there any particular reason we shall allow id again? E.g. incompatibility with some of the open source clients?

[diegorondini]

@avgustinmm
I can list two:

1. hara-ddiclient (Eclipse hawkBit subproject)
2. swupdate: https://github.com/sbabic/swupdate/blob/2023.12/suricatta/server_hawkbit.c#L396

but as @floruschbaschan clarified in his comment the parameter was effectively mandated by hawkBit ≤ 0.3M7, so I expect any client developed before April 2023 (date of release of 0.3M8) to provide that parameter.

Also, DDI APIs are supposed to change version when implementing breaking changes:

However, the device facing DDI API is on major version 'v1' and will be kept stable.

https://github.com/eclipse/hawkbit#status-and-api-stability
Removing a parameter is a breaking change. The correct approach should be to deprecate and ignore it in v1 and then remove it in DDI v2.

[sbabic]

Hi,

On 24.01.24 17:43, Diego Rondini wrote: @avgustinmm <https://github.com/avgustinmm> I can list two: 1. hara-ddiclient (Eclipse hawkBit subproject) 2. swupdate: https://github.com/sbabic/swupdate/blob/2023.12/suricatta/server_hawkbit.c#L396 <https://github.com/sbabic/swupdate/blob/2023.12/suricatta/server_hawkbit.c#L396> but as @floruschbaschan <https://github.com/floruschbaschan> clarified in his comment the parameter was effectively mandated by hawkBit ≤ 0.3M7, so I expect *any client developed before April 2023 (date of release of 0.3M8)* to provide that parameter. Also, DDI APIs are supposed to change version when implementing breaking changes: However, the device facing DDI API <https://github.com/eclipse/hawkbit/tree/master/hawkbit-rest/hawkbit-ddi-api> is on major version 'v1' and will be kept stable.

Fully agree - this is a change in the API and should require a change in the major version. There are plenty of devices in the field and they are connecting with V1 API. As soon as a new hawkbit version will be deployed on the server, all these devices lose OTA capabilities. Regards, Stefano Babic

…

https://github.com/eclipse/hawkbit#status-and-api-stability <https://github.com/eclipse/hawkbit#status-and-api-stability> Removing a parameter is a breaking change. The correct approach should be to deprecate and ignore it in v1 and then remove it in DDI v2. — Reply to this email directly, view it on GitHub <#1565 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAKYXDXVSHDSLXRCXVU5OADYQE23NAVCNFSM6AAAAABCIY3OQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBYGUYTINRWGU>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[diegorondini]

Hi @avgustinmm

I just had a look at RAUC code (hi @ejoerns and @Bastian-Krause), and it looks like they always send the id as well:
https://github.com/rauc/rauc-hawkbit-updater/blob/76f5a1dad2f42dc778d798bc0e68fcfbce690b65/src/hawkbit-client.c#L622
https://github.com/rauc/rauc-hawkbit-updater/blob/76f5a1dad2f42dc778d798bc0e68fcfbce690b65/src/hawkbit-client.c#L561

For all those three clients, we're talking about code that dates back to 2016 - 2018:
sbabic/swupdate@3d88068#diff-011008ec659e2771a0d377d60eeda5a8d3f1bbee7652d614326ce0029d0f61edR239

My suggestion is to restore the id as deprecated and release hawkBit 0.3.1 and 0.4.2 that restore the compatibility with the original DDI v1.

Let us know if you need help with this.

Regards

[avgustinmm]

Have you tested that hawkBit rejects requests with "id"?
The DdiActionFeedback is annotated with @JsonIgnoreProperties(ignoreUnknown = true) which means that it shall just ignore unknown fields of the input payload.
Just tried with 0.4.1 monolith docker image via swagger:

curl -X 'POST' \
  'http://localhost:8080/DEFAULT/controller/v1/ctrlId/deploymentBase/4/feedback' \
  -H 'accept: */*' \
  -H 'Authorization: GatewayToken b6608fe2dd6a9d5235b5e9416a0af64b' \
  -H 'Content-Type: application/json' \
  -d '{
  "id": "igonre_it",
  "time": "2023-08-03T12:31:41.890992967Z",
  "status": {
    "execution": "closed",
    "result": {
      "finished": "success",
      "progress": {
        "cnt": 2,
        "of": 5
      }
    },
    "code": 200,
    "details": [
      "string"
    ]
  }
}'

and got:

A payload with "id" is also successfully deserialized using ObjectMapper.

[diegorondini]

Hi @avgustinmm

thanks for pointing that out. After testing more accurately we can confirm that the id is ignored if provided by the client.

To recap:

1. any hawkbit version (including the latest 0.4.1) will provide a response (HTTP 200 code) if the client provides the id
2. hawkBit ≤ 0.3M7 will reply with an HTTP 404 "Not found" if the client does NOT provide the id
3. hawkBit ≥ 0.3M8 will provide a response (HTTP 200 code) if the client does NOT provide the id

So it is reasonable for clients to keep providing the id to maintain compatibility with hawkBit ≤ 0.3M7 for some more time.

At this point what I suggest is to clarify in the API documentation, maybe with a note, that:

1. the id was in practice mandatory in hawkBit implementation until 0.3M7
2. it is indeed ignored if provided in ≥ 0.3M8

Thank you

[Bastian-Krause]

To recap:

1. any hawkbit version (including the latest 0.4.1) will provide a response (HTTP 200 code) if the client provides the id
2. hawkBit ≤ 0.3M7 will reply with an HTTP 404 "Not found" if the client does NOT provide the id
3. hawkBit ≥ 0.3M8 will provide a response (HTTP 200 code) if the client does NOT provide the id

Speaking from rauc-hawkbit-updater's perspective, that sounds correct. We haven't heard of any issues (although we're always providing the id) and our tests against all of these hawkBit releases work fine.

[diegorondini]

@avgustinmm did you clarify anywhere in the documentation that id was part of the API but has been removed and ignored since 0.3M8?

[avgustinmm]

M* are milestones and I don't see the point of putting them into documentation.
Also, I don't see the need to clarify anything - if the client provides an id - it's accepted. If a developer starts to write integration - he will see that it's not needed and won't put it in the request.

[diegorondini]

M* are milestones and I don't see the point of putting them into documentation.

Well, the site and the documentation were updated when milestones were published.
The id was part of 0.2.x releases which were not milestones, and included in stable DDI APIs documentation.

I guess mentioning the id existed in the past won't hurt anybody, and will help those who have used ≤0.3M7.
Keep in mind:

1. releases ≤0.3M7 require the id
2. 0.3M8 has been released less than a year ago (April '23)