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

feat: 2647.2 add secure template repos to pfe #2747

Conversation

rwalle61
Copy link
Contributor

What type of PR is this ?

  • Bug fix
  • Enhancement

What does this PR do ?

Extends PFE's POST /templates/repositories to accept GHE repos if we provide gitCredentials (username & password) in the request body. See the openapi.yaml and the API tests for more details.

Currently if we successfully add a secure template, then subsequent calls to GET /templates/repositories and GET /templates will show the secure repo and its templates respectively. i.e. we do not check whether the credentials are still valid

We also do not currently encrypt the credentials in the request body, meaning they get logged on trace log level. We will need a solution for that in future

I had to do some refactoring to get my changes in but the tests should cover my changes ~100%

Which issue(s) does this PR fix ?

part 2 of #2647

Does this PR require a documentation change ?

2647 will require documentation changes

Any special notes for your reviewer ?

To run the tests you will need to input your own GHE credentials

@rwalle61 rwalle61 force-pushed the 2647.2-add-secure-template-repos-to-pfe branch 2 times, most recently from aaa0848 to 3bcc568 Compare April 23, 2020 11:19
@codecov
Copy link

codecov bot commented Apr 23, 2020

Codecov Report

Merging #2747 into master will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2747      +/-   ##
==========================================
- Coverage   60.51%   60.48%   -0.04%     
==========================================
  Files         102      102              
  Lines       10318    10304      -14     
  Branches     1739     1739              
==========================================
- Hits         6244     6232      -12     
+ Misses       4074     4072       -2     
Impacted Files Coverage Δ
src/pfe/portal/modules/ExtensionList.js 100.00% <100.00%> (ø)
src/pfe/portal/modules/Templates.js 97.91% <100.00%> (+0.53%) ⬆️
src/pfe/portal/routes/templates.route.js 69.31% <100.00%> (-2.12%) ⬇️
src/pfe/portal/modules/utils/sharedFunctions.js 84.26% <0.00%> (+0.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da05275...dbabda5. Read the comment docs.

@rwalle61 rwalle61 force-pushed the 2647.2-add-secure-template-repos-to-pfe branch from b026283 to 4eac971 Compare April 23, 2020 13:08
Richard Waller added 3 commits April 23, 2020 14:12
@rwalle61 rwalle61 force-pushed the 2647.2-add-secure-template-repos-to-pfe branch from 4eac971 to dbabda5 Compare April 23, 2020 13:12
Copy link
Contributor

@hhellyer hhellyer left a comment

Choose a reason for hiding this comment

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

LGTM - Most of the changes look good but I think there might be a bug with locking and other error handling in templates.route.js now.

Richard Waller added 2 commits April 24, 2020 10:13
@hhellyer hhellyer merged commit 82ea59e into eclipse-archived:master Apr 24, 2020
@rwalle61 rwalle61 deleted the 2647.2-add-secure-template-repos-to-pfe branch April 27, 2020 08:51
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.

2 participants