Skip to content
This repository was archived by the owner on Nov 1, 2023. It is now read-only.

Conversation

@chkeita
Copy link
Contributor

@chkeita chkeita commented Dec 6, 2022

Summary of the Pull Request

Currently the Pool has an optional field for the client_id meant to be used in the unmanaged node scenario to identify the agent. It is associated with an application registration.

This PR replaces it with the object id of the service principal associated with that application registration.
This gives us more flexibility giving us the ability to use a managed identity as the authentication method in the unmanaged case.

This PR also updates the authentication logic to also include the reason for rejecting a request.
The agent logic is also updated to give the content of the response body when a request fails.

Note: The client_id can safely be removed from the pool record because it is not currently used

@codecov-commenter
Copy link

codecov-commenter commented Dec 6, 2022

Codecov Report

Merging #2673 (c0d449f) into main (fa3e6a4) will decrease coverage by 0.20%.
The diff coverage is 63.85%.

@@            Coverage Diff             @@
##             main    #2673      +/-   ##
==========================================
- Coverage   29.31%   29.10%   -0.21%     
==========================================
  Files         291      284       -7     
  Lines       36166    34550    -1616     
==========================================
- Hits        10603    10057     -546     
+ Misses      25563    24493    -1070     
Impacted Files Coverage Δ
...iService/ApiService/Functions/AgentRegistration.cs 85.34% <ø> (ø)
src/ApiService/ApiService/ServiceConfiguration.cs 0.00% <0.00%> (ø)
...ice/ApiService/onefuzzlib/EndpointAuthorization.cs 22.13% <0.00%> (+0.35%) ⬆️
...ApiService/ApiService/onefuzzlib/PoolOperations.cs 40.00% <50.00%> (ø)
src/agent/reqwest-retry/src/lib.rs 87.59% <88.46%> (+2.91%) ⬆️
src/ApiService/ApiService/Functions/Pool.cs 92.63% <100.00%> (ø)
src/ApiService/ApiService/OneFuzzTypes/Model.cs 71.07% <100.00%> (ø)
src/ApiService/ApiService/OneFuzzTypes/Requests.cs 62.80% <100.00%> (ø)
...rc/ApiService/ApiService/OneFuzzTypes/Responses.cs 74.30% <100.00%> (ø)
src/agent/onefuzz/src/env.rs 0.00% <0.00%> (-83.34%) ⬇️
... and 32 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@chkeita chkeita requested review from Porges and tevoinea and removed request for tevoinea December 12, 2022 16:55
@chkeita chkeita marked this pull request as ready for review December 12, 2022 18:01
Copy link
Member

@tevoinea tevoinea left a comment

Choose a reason for hiding this comment

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

Just the column rename question

@chkeita chkeita enabled auto-merge (squash) December 13, 2022 03:30
@chkeita chkeita merged commit 4c1adb6 into microsoft:main Dec 13, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jan 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants