Refactor BootstrapConfigStore to use a list of "request" #883

sbernard31 · 2020-08-31T17:22:38Z

This aims to implements : #437

sbernard31 · 2020-09-04T09:45:54Z

First thought about this :

Currently, to define what should be done during a bootstrap session, we need to create a BootstrapConfig.

This is a bean in which you are able to define this actions in this order :

delete anything,
write security objects,
write server objects,
write ACLs.

This has several limitation like :

you can not choose the order of the requests,
you can not write something else than security/server/acl,
if models change you need to upgrade/change the BootstrapConfig bean and probably serialization.

So replacing BootstrapConfig bean by a list of BootstrapDownlinkRequest sounds to resolve all those limitations, but this brings new ones :

This makes bootstrap config validation very difficult. (it sounds difficult to have an equivalent for ConfigurationChecker)
Creating LwM2mNode content for WriteRequest is more painful, complicated and error prone than creating BootstrapConfig

So even if this is not so much satisfying to me, maybe we should keep both API...
For issue 2. I also try a kind of experimental API to make more simple LwM2node creation.
At usage it looks like this : 7db5373#diff-cdf3b6c29cc5769b5054f2d8a660d7afR465

Working on this makes me think about other issues relative to Bootstrap server :

Object Model Versioning : unlike to registration, devices does not begin bootstrap session by saying which version it supports. This could be a problem to found object models to use. (Actually I'm not sure if this is a real problem, if object models are backward compatible maybe have the last version of the object is enough)
=> If a solution is needed, we could imagine to start session with a Bootstrap discover request to know which version to use.
If bootstrap server want to apply a config dynamically regarding of device states using Discover or Read request this is currently not possible.
=> Maybe a solution could be that BootstrapConfigurationStore can return read/discover requests OR delete/write requests. If read/discover is returned, once we get the result we call again BootstrapConfigurationStore with read/discover responses in parameter. If delete/write is returned, once we apply all requests we send a bootstrap finished.

sbernard31 · 2020-09-21T16:23:27Z

@dachaac do you have any opinions on this ?

dachaac · 2020-09-21T17:01:25Z

@sbernard31 kinda sounds into right direction and there might be a needs in certain installations to do custom object writing during bootstrapping -- at least it gives flexibility. What one need to make sure that all operations has been done successfully before bootstrap finish is issued.

About the implementation you have here -- didn't have time to look at it yet -- I will rebase my work on the master now that unsigned integer is there and push it for you for a preview on how I tried to go in between the bootstrapping process and connecting to lwm2m server. With that done we can see if there is something that should be taken into account and I can hopefully give some more though on this issue then.

dachaac · 2020-09-21T17:33:04Z

@sbernard31 rebased my https://github.com/dachaac/leshan/commits/est-support branch.

It still needs tidying up and has some traces of ugliness while hacking in ;) and is missing CoAP-EST server stuff as I am wondering currently how to split it and where and what kind of interfaces would be good -- but I would say that is not related to this issue as such.

sbernard31 · 2020-09-22T14:14:40Z

About the implementation you have here -- didn't have time to look at it yet

No problem I was mainly interested about your opinion on the idea itself and the new issues it could bring. (The code is currently no so clean it's more a kind of draft)

sbernard31 · 2020-10-06T16:14:33Z

I cleaned my code and limit the scope of this PR by letting the bsserver-demo untouched.

The new BootstrapConfigurationStore replace the old BootstrapConfigStore(deprecated now).
About backward compatibility, BootstrapConfigurationStoreAdapter can be used.

About API issues raised in #883 (comment)
If we still want to keep the old BootstrapConfig bean for convenience, there is a BootstrapUtil utility to facilitate conversion to requests.
Another solution could be to explore a new API like : this experimental LwM2mNodeBuilder ...

About bsserver-demo, I didn't touch it for now, meaning it is using lot of deprecated stuff...

sbernard31 · 2020-10-06T16:15:10Z

I think this is in reviewable state now.

dachaac · 2020-10-07T19:37:53Z

Lots of changes were just change from on way to another "in cosmetic category". Actual change seems to be the FIFO that gets generated from BootstrapConfig which is probably OK but then again you had BootstrapConfiguration which was planned to replace it?

I am not sure if BootstrapConfiguration is actually serializable in case it needs to be stored in database for persistence?

Or should this just be runtime thing and it gets generated from other config/code?

sbernard31 · 2020-10-09T13:05:51Z

I am not sure if BootstrapConfiguration is actually serializable in case it needs to be stored in database for persistence?

This is not serializable as java.io.Serializable meaning, but this is just a list of bean so we should be able to serialize this list of requests.

or should this just be runtime thing and it gets generated from other config/code?

Both are possible.
E.g : In our case in production we don't persist BootstrapConfig or BootstrapConfiguration we create it from data extracted from different places

Lots of changes were just change from on way to another "in cosmetic category". Actual change seems to be the FIFO that gets generated from BootstrapConfig which is probably OK but then again you had BootstrapConfiguration which was planned to replace it?

My first idea was to replace BootstrapConfiguration by BootstrapConfig to bring more flexibility.
But this brings new issues (see #883 (comment))
So for now I didn't get rid off old BootstrapConfig (but if we can I would prefer) or maybe we need both, one for convenience on main bootstrap objects (security, server) and the other for all other objects.

dachaac · 2020-10-09T13:27:54Z

Ok. I don't see problems with the changes in here.

Just a comment on deprecated stuff -- Both LWM2M Server Demo and LWM2M Bootstrap Server Demo has a Web UI. They are built with different frameworks.

LWM2M Server is utilizing AngularJS which is going for EOL:
https://blog.angular.io/stable-angularjs-and-long-term-support-7e077635ee9c

LWM2M Bootstrap server then again is utilizing Riot.

On my other changes I just tried to adapt what ever was used but was a bit puzzled why the difference.

Do you have some plans on this sector?

sbernard31 · 2020-10-09T13:44:54Z

Both LWM2M Server Demo and LWM2M Bootstrap Server Demo has a Web UI. They are built with different frameworks.

As demos are not the priority, we don't invest as many time we would like.
There is a lot of technical debt and the more we add features to it the more it is hard to migrate.
I will create a dedicated issue about current identified problem with our demos.

why the difference.

This is just historical reasons + lack of time.
Angular was chosen at the beginning, I don't know why (I was not in the project at this time) but the team was really disappointing by it. So when we create the bootstrap server demo we try something else : Riot.
My personal feeling is Riot is not so bad, but maybe not enough popular and so could be an obstacle for contribution.

sbernard31 · 2020-10-09T14:15:07Z

see #907

msangoi

LGTM

After more thoughts, we decide that this was not the good approach. See #437 (comment) for more details. Consequences BootstrapConfig is no more deprecated.

sbernard31 changed the base branch from master to jackson_decoder August 31, 2020 17:23

sbernard31 force-pushed the jackson_decoder branch from 2a71d79 to 524c631 Compare September 4, 2020 08:42

sbernard31 force-pushed the bootstrap_refactoring branch from 1b8da2f to 7db5373 Compare September 4, 2020 08:42

This was referenced Sep 4, 2020

Roadmap to EST support #859

Closed

Support for communication to the bootstrap server using OSCORE #857

Closed

sbernard31 force-pushed the jackson_decoder branch from 524c631 to 9dd235a Compare September 14, 2020 09:36

sbernard31 force-pushed the bootstrap_refactoring branch from 7db5373 to d80524c Compare September 14, 2020 09:45

sbernard31 changed the base branch from jackson_decoder to master September 14, 2020 09:45

sbernard31 mentioned this pull request Sep 29, 2020

Certificate usage support #896

Closed

sbernard31 force-pushed the bootstrap_refactoring branch from d80524c to d19212a Compare October 6, 2020 15:41

sbernard31 requested a review from msangoi October 6, 2020 16:15

sbernard31 mentioned this pull request Oct 9, 2020

leshan-demo technical debt #907

Closed

msangoi approved these changes Oct 29, 2020

View reviewed changes

#437: Add a BootstrapConfigurationStore which stores BootstrapRequets.

3001a33

sbernard31 force-pushed the bootstrap_refactoring branch from d19212a to 3001a33 Compare November 2, 2020 09:26

sbernard31 merged commit 3001a33 into master Nov 2, 2020

sbernard31 mentioned this pull request Nov 2, 2020

Enable client bootstrapping to receive OSCORE security information #919

Closed

sbernard31 mentioned this pull request Dec 18, 2020

Support client bootstrapping to receive OSCORE security information #950

Closed

sbernard31 deleted the bootstrap_refactoring branch January 26, 2021 16:03

This was referenced Jun 10, 2021

"BootstrapConfig" should be a list of request #437

Closed

Refactoring Bootstrap Server (again) #1027

Merged

sbernard31 added a commit that referenced this pull request Jun 10, 2021

#1027: remove BootstrapConfiguration added by #883

0bdcbc8

After more thoughts, we decide that this was not the good approach. See #437 (comment) for more details. Consequences BootstrapConfig is no more deprecated.

sbernard31 added a commit that referenced this pull request Jun 25, 2021

#1027: remove BootstrapConfiguration added by #883

f68537d

After more thoughts, we decide that this was not the good approach. See #437 (comment) for more details. Consequences BootstrapConfig is no more deprecated.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor BootstrapConfigStore to use a list of "request" #883

Refactor BootstrapConfigStore to use a list of "request" #883

sbernard31 commented Aug 31, 2020 •

edited

Loading

sbernard31 commented Sep 4, 2020

sbernard31 commented Sep 21, 2020

dachaac commented Sep 21, 2020

dachaac commented Sep 21, 2020

sbernard31 commented Sep 22, 2020

sbernard31 commented Oct 6, 2020

sbernard31 commented Oct 6, 2020

dachaac commented Oct 7, 2020

sbernard31 commented Oct 9, 2020

dachaac commented Oct 9, 2020

sbernard31 commented Oct 9, 2020

sbernard31 commented Oct 9, 2020

msangoi left a comment

Refactor BootstrapConfigStore to use a list of "request" #883

Refactor BootstrapConfigStore to use a list of "request" #883

Conversation

sbernard31 commented Aug 31, 2020 • edited Loading

sbernard31 commented Sep 4, 2020

sbernard31 commented Sep 21, 2020

dachaac commented Sep 21, 2020

dachaac commented Sep 21, 2020

sbernard31 commented Sep 22, 2020

sbernard31 commented Oct 6, 2020

sbernard31 commented Oct 6, 2020

dachaac commented Oct 7, 2020

sbernard31 commented Oct 9, 2020

dachaac commented Oct 9, 2020

sbernard31 commented Oct 9, 2020

sbernard31 commented Oct 9, 2020

msangoi left a comment

Choose a reason for hiding this comment

sbernard31 commented Aug 31, 2020 •

edited

Loading