Skip to content

refactor(fdo): re-enable credential reuse logic#1909

Closed
AliouneGaye21 wants to merge 0 commit intoastarte-platform:baofrom
AliouneGaye21:bao11
Closed

refactor(fdo): re-enable credential reuse logic#1909
AliouneGaye21 wants to merge 0 commit intoastarte-platform:baofrom
AliouneGaye21:bao11

Conversation

@AliouneGaye21
Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Additional documentation e.g. usage docs, diagrams, reviewer notes, etc.:


Thanks for sending a pull request! If this is your first time, here are some tips for you:
  1. You can take a look at our developer guide for an introduction on Astarte development!
  2. Make sure to read CONTRIBUTING.md and CODE_OF_CONDUCT.md
  3. If the PR is unfinished or you're actively working on it, mark it as draft

When fixing existing issues, use github's syntax to link your pull request to it

fixes #<issue number>

We also have a syntax to signal dependencies to other open pull requests

depends on #<pr number>
depends on https://github.com/...

In case of stacked PRs, you may add the PR number in the last commit's title instead:

gitGraph
    commit id: "Current master"
    branch feat1
    checkout feat1
    commit id: "feat: add something"
    commit id: "feat: add something else (#100)"
    branch feat2
    checkout feat2
    commit id: "refactor: do something"
    commit id: "fix: solve issue"
    commit id: "feat: add a feature (#101)"
    branch feat3
    checkout feat3
    commit id: "feat: feat without pr number"
Loading

@github-actions github-actions bot added the size/s label Apr 3, 2026
@AliouneGaye21 AliouneGaye21 requested a review from noaccOS April 3, 2026 15:28
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 3, 2026

Codecov Report

❌ Patch coverage is 70.00000% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.68%. Comparing base (d619fbc) to head (e146b59).
⚠️ Report is 1 commits behind head on bao.

Files with missing lines Patch % Lines
libs/astarte_fdo/lib/owner_onboarding.ex 50.00% 6 Missing ⚠️
.../astarte_fdo/lib/ownership_voucher/load_request.ex 77.27% 5 Missing ⚠️
libs/astarte_secrets/lib/astarte_secrets/core.ex 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              bao    #1909      +/-   ##
==========================================
+ Coverage   84.64%   84.68%   +0.04%     
==========================================
  Files         385      394       +9     
  Lines        8003     8339     +336     
==========================================
+ Hits         6774     7062     +288     
- Misses       1229     1277      +48     
Files with missing lines Coverage Δ
...s/lib/astarte_data_access/fdo/ownership_voucher.ex 100.00% <ø> (ø)
libs/astarte_fdo/lib/owner_onboarding/session.ex 84.74% <ø> (ø)
libs/astarte_secrets/lib/astarte_secrets/core.ex 92.00% <83.33%> (ø)
.../astarte_fdo/lib/ownership_voucher/load_request.ex 86.02% <77.27%> (-0.65%) ⬇️
libs/astarte_fdo/lib/owner_onboarding.ex 79.12% <50.00%> (+3.21%) ⬆️

... and 13 files with indirect coverage changes

Flag Coverage Δ
astarte_appengine_api 85.73% <ø> (ø)
astarte_data_access 81.94% <ø> (ø)
astarte_data_updater_plant 87.13% <ø> (ø)
astarte_events 80.27% <ø> (ø)
astarte_fdo 81.65% <67.64%> (+0.92%) ⬆️
astarte_fdo_core 61.50% <ø> (ø)
astarte_generators 97.39% <ø> (-0.35%) ⬇️
astarte_housekeeping 85.52% <ø> (ø)
astarte_realm_management 87.66% <ø> (+0.15%) ⬆️
astarte_rpc 88.23% <ø> (ø)
astarte_secrets 85.16% <83.33%> (?)
astarte_trigger_engine 82.52% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

def credential_reuse?(session) do
is_nil(session.replacement_pub_key) and
is_nil(session.replacement_rv_info) and
session.replacement_guid == session.guid
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
session.replacement_guid == session.guid
is_nil(session.replacement_guid)

except it's not the session, all 3 values are in the ownership voucher entry

Comment on lines +143 to +152
next_guid = session.replacement_guid || guid
next_rv_info = session.replacement_rv_info || ownership_voucher.header.rendezvous_info
next_owner_pub_key = session.replacement_pub_key || owner_public_key

# {:ok, private_key} = COSE.Keys.from_pem(private_key)

connection_credentials = %{
guid: guid,
rendezvous_info: rendezvous_info,
owner_pub_key: owner_public_key,
guid: next_guid,
rendezvous_info: next_rv_info,
owner_pub_key: next_owner_pub_key,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's revert this: the session is for that single guid, replacement data is in the ownership_voucher entry instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants