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

🎨 efs improvements (group extra properties) πŸ—ƒοΈ #6493

Merged

Conversation

matusdrobuliak66
Copy link
Contributor

@matusdrobuliak66 matusdrobuliak66 commented Oct 7, 2024

What do these changes do?

  • πŸ—ƒοΈ Add enabled_efs to the groups_extra_properties database table.
  • ♻️ Run EFS Guardian with root access.
    • EFS Guardian requires elevated privileges:
      • Currently, user services start as root and override all permissions. Additionally, all new files are created by the user defined by the service. EFS Guardian needs access to these files so it can compute their size and, in special cases, revoke write permissions.
      • Guardian also needs permission to delete files that are no longer in use for a specified period of time.

My strategy:

  1. Once this PR is merged, our users can start using it immediately.
  2. We need to implement two background tasks in EFS Guardian:
  • One to monitor storage size and change permissions when the limit is reached.
  • Another to monitor and remove files that have not been used for an extended period.
  1. We can enhance ownership management to ensure EFS doesn't run as root.

Related issue/s

How to test

Dev-ops checklist

Copy link

codecov bot commented Oct 7, 2024

Codecov Report

All modified and coverable lines are covered by tests βœ…

Project coverage is 89.2%. Comparing base (cafbf96) to head (ee25042).
Report is 619 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #6493      +/-   ##
=========================================
+ Coverage    84.5%   89.2%    +4.7%     
=========================================
  Files          10    1329    +1319     
  Lines         214   55436   +55222     
  Branches       25     961     +936     
=========================================
+ Hits          181   49495   +49314     
- Misses         23    5788    +5765     
- Partials       10     153     +143     
Flag Coverage Ξ”
integrationtests 64.7% <100.0%> (?)
unittests 86.9% <75.0%> (+2.4%) ⬆️

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

Files with missing lines Coverage Ξ”
...ostgres_database/models/groups_extra_properties.py 100.0% <ΓΈ> (ΓΈ)
...postgres_database/utils_groups_extra_properties.py 95.9% <100.0%> (ΓΈ)
...modules/db/repositories/groups_extra_properties.py 86.3% <100.0%> (ΓΈ)
...es/dynamic_sidecar/docker_service_specs/sidecar.py 88.0% <100.0%> (ΓΈ)
..._sidecar/scheduler/_core/_event_create_sidecars.py 100.0% <100.0%> (ΓΈ)

... and 1289 files with indirect coverage changes

@matusdrobuliak66 matusdrobuliak66 self-assigned this Oct 7, 2024
Copy link

sonarqubecloud bot commented Oct 7, 2024

@matusdrobuliak66 matusdrobuliak66 added this to the MartinKippenberger milestone Oct 7, 2024
@matusdrobuliak66 matusdrobuliak66 marked this pull request as ready for review October 7, 2024 09:59
@matusdrobuliak66 matusdrobuliak66 requested a review from GitHK October 7, 2024 12:53
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. Please consider my suggestions before merging

services/efs-guardian/Dockerfile Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants