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

✨oSPARC API keys are created and removed if a service requires them #5004

Merged

Conversation

GitHK
Copy link
Contributor

@GitHK GitHK commented Nov 9, 2023

What do these changes do?

DISCLAIMER: This PR, being small, is more noisy than I would have preferred.

Makes sure that for every each run there are different oSPARC API keys for each service which requires them.

  • Creates API keys when a service requires them, ONLY if they are requested.
  • Removes create API keys when the service shuts down, ONLY if they were created.

Changes per service:

packages:

  • ✨ added BaseOsparcGenericResourceManager. The user only needs to create read and delete and the resource lifecycle is simplified.
  • ♻️ ApiKeyInDB moved from api-server to models-library

director-v2:

  • director-v2 creates and removes api keys, if required, whenever a service is started and shut down
  • ✨ implemented resolve_and_substitute_lifespan_variables_in_specs and now used when starting dynamic-services

webserver:

  • 🐛 fixed webserver's web API spec
  • ✨ added new RPC route to webserver to get an existing API key
  • ✨ added tests for the RPC part of the API Keys inside webserver
  • code to extend the API for getting an api key Extend webserver's ApiKey API #5022

Related issue/s

How to test

DevOps Checklist

@GitHK GitHK self-assigned this Nov 9, 2023
@GitHK GitHK added this to the 7peaks milestone Nov 9, 2023
@GitHK GitHK added the a:director-v2 issue related with the director-v2 service label Nov 9, 2023
Copy link

codecov bot commented Nov 9, 2023

Codecov Report

Merging #5004 (f8f6a4d) into master (cd84f2f) will increase coverage by 0.1%.
The diff coverage is 84.4%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #5004     +/-   ##
========================================
+ Coverage    87.2%   87.4%   +0.1%     
========================================
  Files        1250    1047    -203     
  Lines       51237   46090   -5147     
  Branches     1107     540    -567     
========================================
- Hits        44699   40297   -4402     
+ Misses       6299    5672    -627     
+ Partials      239     121    -118     
Flag Coverage Δ
integrationtests 65.0% <71.2%> (-0.1%) ⬇️
unittests 84.9% <74.8%> (-0.1%) ⬇️

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

Files Coverage Δ
.../models_library/api_schemas_api_server/api_keys.py 100.0% <100.0%> (ø)
...rc/simcore_service_director_v2/core/application.py 97.4% <100.0%> (+<0.1%) ⬆️
...v2/modules/dynamic_sidecar/docker_compose_specs.py 97.9% <100.0%> (+<0.1%) ⬆️
...service_director_v2/utils/distributed_identifer.py 100.0% <100.0%> (ø)
...core_service_director_v2/utils/osparc_variables.py 95.5% <100.0%> (+<0.1%) ⬆️
...ver/src/simcore_service_webserver/api_keys/_api.py 100.0% <100.0%> (ø)
...rver/src/simcore_service_webserver/api_keys/_db.py 94.3% <100.0%> (+1.0%) ⬆️
...ver/src/simcore_service_webserver/api_keys/_rpc.py 100.0% <100.0%> (+11.1%) ⬆️
...re_service_director_v2/modules/api_keys_manager.py 87.5% <87.5%> (ø)
...s/dynamic_sidecar/scheduler/_core/_events_utils.py 92.3% <61.1%> (-2.5%) ⬇️
... and 1 more

... and 210 files with indirect coverage changes

…HK/osparc-simcore-forked into pr-osparc-support-api-keyreplacement
Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

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

i do not want to delay this more ... but I think this PR will have an unnecessary maintenance overhead :-(

@GitHK GitHK requested a review from sanderegg November 15, 2023 10:45
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

go for it and thanks for the changes and talk!

Copy link

codeclimate bot commented Nov 15, 2023

Code Climate has analyzed commit f8f6a4d and detected 0 issues on this pull request.

View more on Code Climate.

Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.1% 0.1% Duplication

@GitHK GitHK merged commit 633ec55 into ITISFoundation:master Nov 15, 2023
55 checks passed
@GitHK GitHK deleted the pr-osparc-support-api-keyreplacement branch November 15, 2023 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:director-v2 issue related with the director-v2 service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants