fix(deviceauth): Allow new authset for accepted devices at max limit - #2055
fix(deviceauth): Allow new authset for accepted devices at max limit#2055bahaa-ghazal wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
merlin-northern
left a comment
merlin-northern
left a comment
There was a problem hiding this comment.
I would really love an expicit test scenarion for this in the tests-ng, doable?
| dev.AuthSets = []model.AuthSet{*aset} | ||
|
|
||
| reqId := requestid.FromContext(ctx) | ||
| if !deviceAlreadyAccepted { |
There was a problem hiding this comment.
cold you double check, I understand the change, in here we used to start the provision no matter what, and if the device had been accepted and we set the auth set status to accepted this seems not necessary to start the provision workflow, could you doble check that we for sure do not rely on that? I do not think so, but this function is a huge trap.
There was a problem hiding this comment.
The only case I can think of that this might break is that if we try to accept an authset for a existing device but with different tier then the one accepted. Do we allow that change of tiers for accepted device? or the device must be rejected first then re-accepted with the new tier?
There was a problem hiding this comment.
tiers do not exist in os, so on this you have to fall in ent. but the tier change works by accepting a new authset, and yes, the device stays accepted.
There was a problem hiding this comment.
you know what, it wont hurt do add provision_device call in OS. we need that in enterprise because it used the provision_device_v2 which gets attributes as param to the workflow. we can just keep the first part of skipping counting limits if device is accepted
There was a problem hiding this comment.
I hate the sync ... the enterprise would need a different fix because of tiers
Ticket: MEN-9955 Signed-off-by: Bahaa Aldeen Ghazal <bahaa.ghazal@northern.tech>
3 participants
Ticket: MEN-9955
Will add ng test in enterprise, since there is no limits is OS