-
Notifications
You must be signed in to change notification settings - Fork 44
feat: POST /templates/repos accepts Personal Access Tokens #2762
feat: POST /templates/repos accepts Personal Access Tokens #2762
Conversation
Signed-off-by: Richard Waller <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2762 +/- ##
==========================================
+ Coverage 60.51% 60.56% +0.05%
==========================================
Files 102 102
Lines 10337 10340 +3
Branches 1745 1747 +2
==========================================
+ Hits 6255 6262 +7
+ Misses 4082 4078 -4
Continue to review full report at Codecov.
|
Signed-off-by: Richard Waller <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - Might need additional check for valid combinations of credentials.
options.headers = { | ||
Authorization: `token ${gitCredentials.personalAccessToken}`, | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should probably be an extra check to confirm the user hasn't either sent all 3 when they've sent a username and password and or sent an empty git credentials structure (or just one of username and password).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed the openapi.yml makes it hard to hit these from outside but I think we should probably still have the check as we also load repository configuration from our saved repository file when we start up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea the OpenAPI yaml should do that validation for calls from the outside.
I can add checks here too, but we don't yet support the case where the saved repository file tries to add secure templates to PFE. For example that file currently doesn't save credentials, so can't add secure templates to PFE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added the checks in a further commit
Signed-off-by: Richard Waller <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - Thanks for adding the validation.
Signed-off-by: Richard Waller [email protected]
What type of PR is this ?
What does this PR do ?
Extends POST /templates/repos to accept GitHub Personal Access Tokens when adding tetmplate repos
Which issue(s) does this PR fix ?
part of #2647
Does this PR require a documentation change ?
Included
Any special notes for your reviewer ?