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

⚗️ Add password authentication for redis ( ⚠️ OPS) #6035

Merged
merged 38 commits into from
Jul 22, 2024

Conversation

mrnicegyu11
Copy link
Member

@mrnicegyu11 mrnicegyu11 commented Jul 9, 2024

** Follow-Up PR for #5708**

What do these changes do?

We would like to use the AWS managed, fully redis-compatible "MemoryDB" in AWS, since we have now introduced non-ephemeral data in redis that should strictly persist even across incidents (CC @GitHK for details). AWS provides MemoryDB (see https://docs.aws.amazon.com/memorydb/latest/devguide/what-is-memorydb-for-redis.html ) as an endpoint/URL, that redis-clients can connect to with password authentication. Up until now, redis was running inside the simcore docker stack, isolated from outside connections via docker networks, and simcore services would connect to it directly, unencrypted and without authentication. While there was some infrastructure in the code to allow password authentication, it was not working out-of-the-box.

This PR will change the communication of simcore services with redis to always use password authentication.

Bonus:

  • Sets the timeout of the integration test for the webserver-exporter to 60s (before: 10s), since it would on my machine always need at least 30s timeout to manage to export the project, and since we are not using the exporter anywhere. My hope is that this reduces flakyness of this test. I can revert this bonus-change if needed.

Related issue/s

Related PRs

Dev-ops checklist

@mrnicegyu11 mrnicegyu11 changed the title 2024/07/redis password ⚗️ Add password for redis Jul 9, 2024
@mrnicegyu11 mrnicegyu11 mentioned this pull request Jul 9, 2024
1 task
Copy link

codecov bot commented Jul 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.1%. Comparing base (cafbf96) to head (6107b45).
Report is 361 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #6035      +/-   ##
=========================================
+ Coverage    84.5%   88.1%    +3.5%     
=========================================
  Files          10    1444    +1434     
  Lines         214   59426   +59212     
  Branches       25    1409    +1384     
=========================================
+ Hits          181   52377   +52196     
- Misses         23    6753    +6730     
- Partials       10     296     +286     
Flag Coverage Δ
integrationtests 64.7% <100.0%> (?)
unittests 86.2% <100.0%> (+1.6%) ⬆️

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

Files Coverage Δ
...re_service_webserver/application_settings_utils.py 98.0% <100.0%> (ø)

... and 1403 files with indirect coverage changes

@mrnicegyu11 mrnicegyu11 self-assigned this Jul 9, 2024
mrnicegyu11 and others added 22 commits July 10, 2024 14:04
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Merged on behalf of @bisgaard-itis . Needed for staging release
@mrnicegyu11 mrnicegyu11 changed the title ⚗️ Add password for redis ⚗️ Add password authentication for redis Jul 18, 2024
@mrnicegyu11 mrnicegyu11 added a:infra+ops maintenance of infrastructure or operations (discussed in retro) t:enhancement Improvement or request on an existing feature security Pull requests that address a security vulnerability labels Jul 18, 2024
@mrnicegyu11 mrnicegyu11 added this to the Tom Bombadil milestone Jul 18, 2024
@mrnicegyu11 mrnicegyu11 marked this pull request as ready for review July 19, 2024 14:05
@YuryHrytsuk YuryHrytsuk changed the title ⚗️ Add password authentication for redis ⚗️ Add password authentication for redis (:warning: OPS) Jul 19, 2024
@YuryHrytsuk YuryHrytsuk changed the title ⚗️ Add password authentication for redis (:warning: OPS) ⚗️ Add password authentication for redis ( ⚠️ OPS) Jul 19, 2024
Copy link
Contributor

@YuryHrytsuk YuryHrytsuk left a comment

Choose a reason for hiding this comment

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

Thanks!

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.

Thx. Left some suggestions . please consider them before merging.

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.

thanks, added a few comments, but looks good.

services/docker-compose.yml Show resolved Hide resolved
@mrnicegyu11 mrnicegyu11 merged commit ae9c51b into ITISFoundation:master Jul 22, 2024
56 checks passed
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Aug 15, 2024
67 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:infra+ops maintenance of infrastructure or operations (discussed in retro) security Pull requests that address a security vulnerability t:enhancement Improvement or request on an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants