Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cmd-api-server): plugins interfere with API server deps #1192 #1204

Merged
merged 5 commits into from
Aug 13, 2021

Conversation

petermetz
Copy link
Contributor

@petermetz petermetz commented Aug 11, 2021

Migrates to the lmify package to install plugins at runtime
instead of doing it via vanilla npm which was causing problems
with conflicting dependency versions where the API server would
want semver 7.x and one of the plugins (through some transient
dependency of the plugin itself) would install semver 5.x which
would then cause the API server to break down at runtime due to
the breaking changes between semver 7 and 5.

The magic sauce is the --prefix option of npm which, when specified
instructs npm to ignore the usual parent directory traversal algorithm
when evaluating/resolving dependency trees and instead just do a full
installation to the specified directory path as dictated by the
--prefix option. This means that we can install each plugin in their
own directory the code being isolated from the API server and also
from other plugins that might also interfere.

Fixes hyperledger#1192

Depends on hyperledger#1203

Signed-off-by: Peter Somogyvari peter.somogyvari@accenture.com

@petermetz petermetz added bug Something isn't working API_Server dependencies Pull requests that update a dependency file Security Related to existing or potential security vulnerabilities labels Aug 11, 2021
@petermetz petermetz added this to the v0.8.0 milestone Aug 11, 2021
@petermetz petermetz requested a review from takeutak August 11, 2021 00:28
@petermetz petermetz self-assigned this Aug 11, 2021
@codecov-commenter
Copy link

codecov-commenter commented Aug 11, 2021

Codecov Report

Merging #1204 (2c9f4d9) into main (7b424d4) will decrease coverage by 0.02%.
The diff coverage is 82.14%.

❗ Current head 2c9f4d9 differs from pull request most recent head 4b4a066. Consider uploading reports for the commit 4b4a066 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1204      +/-   ##
==========================================
- Coverage   71.98%   71.95%   -0.03%     
==========================================
  Files         270      270              
  Lines        9440     9438       -2     
  Branches     1113     1112       -1     
==========================================
- Hits         6795     6791       -4     
- Misses       2072     2075       +3     
+ Partials      573      572       -1     
Impacted Files Coverage Δ
...erver/src/main/typescript/config/config-service.ts 74.41% <ø> (ø)
...s-cmd-api-server/src/main/typescript/api-server.ts 86.77% <82.14%> (+1.25%) ⬆️
...ain/typescript/plugin-factory-consortium-manual.ts 75.00% <0.00%> (-25.00%) ⬇️
...ory/src/main/typescript/plugin-factory-keychain.ts 66.66% <0.00%> (-16.67%) ⬇️
...n-keychain-vault/src/main/typescript/public-api.ts 85.71% <0.00%> (-14.29%) ⬇️
...-keychain-memory/src/main/typescript/public-api.ts 85.71% <0.00%> (-14.29%) ⬇️
...onsortium-manual/src/main/typescript/public-api.ts 90.90% <0.00%> (-9.10%) ⬇️
...typescript/plugin-keychain-vault-remote-adapter.ts 61.53% <0.00%> (-2.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 7b424d4...4b4a066. Read the comment docs.

Copy link
Contributor

@jonathan-m-hamilton jonathan-m-hamilton left a comment

Choose a reason for hiding this comment

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

LGTM

…er-cacti#1192

Migrates to the lmify package to install plugins at runtime
instead of doing it via vanilla npm which was causing problems
with conflicting dependency versions where the API server would
want semver 7.x and one of the plugins (through some transient
dependency of the plugin itself) would install semver 5.x which
would then cause the API server to break down at runtime due to
the breaking changes between semver 7 and 5.

The magic sauce is the --prefix option of npm which, when specified
instructs npm to ignore the usual parent directory traversal algorithm
when evaluating/resolving dependency trees and instead just do a full
installation to the specified directory path as dictated by the
--prefix option. This means that we can install each plugin in their
own directory the code being isolated from the API server and also
from other plugins that might also interfere.

Fixes hyperledger-cacti#1192

Depends on hyperledger-cacti#1203

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
@petermetz petermetz merged commit a96ce68 into hyperledger-cacti:main Aug 13, 2021
@petermetz petermetz deleted the fix-1192 branch August 13, 2021 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API_Server bug Something isn't working dependencies Pull requests that update a dependency file Security Related to existing or potential security vulnerabilities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix(cmd-api-server): plugins interfere with API server deps
4 participants