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.

Sign up for GitHub

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

Jump to bottom

NC | Change Check Access Behavior #8359

Merged
merged 1 commit into from
Sep 24, 2024
Merged

NC | Change Check Access Behavior #8359

merged 1 commit into from
Sep 24, 2024

Conversation

romayalon
Copy link
Contributor

@romayalon romayalon commented Sep 12, 2024 •
edited
Loading

Explain the changes

  1. Added 2 new access check related configuration - NC_DISABLE_HEALTH_ACCESS_CHECK, NC_DISABLE_POSIX_MODE_ACCESS_CHECK.
  2. is_dir_rw_accessible() - Skipped mode bits check if NC_DISABLE_POSIX_MODE_ACCESS_CHECK is false.
  3. Health - checked NC_DISABLE_ACCESS_CHECK && NC_DISABLE_HEALTH_ACCESS_CHECK before running stat/is_dir_rw_accessible() on account's health check - Notice it'll skip also the existence check of the account's new_buckets_path.
  4. Fixed a bug in Health - removed the .name since we now return the names list (was changed in the latest refactoring).
  5. We need to consider adding a check for NC_DISABLE_ACCESS_CHECK && NC_DISABLE_HEALTH_ACCESS_CHECK before running stat/is_dir_rw_accessible() on bucket's health check.
  6. Updated docs and existing tests.

Issues: Fixed #xxx / Gap #xxx

  1. Add health tests - found an issue there, will fix it on a different PR.
  2. Fixed #7794 for ACLs (still supplemental groups are not supported)

Testing Instructions:

  • Doc added/updated
  • Tests added

@romayalon romayalon force-pushed the romy-check-access-configuration branch 2 times, most recently from 0126e22 to 9de2852 Compare September 15, 2024 12:57
@romayalon romayalon force-pushed the romy-check-access-configuration branch from 47fc8a2 to a8c3fc7 Compare September 16, 2024 15:03
@romayalon romayalon force-pushed the romy-check-access-configuration branch 2 times, most recently from 054fee6 to d4bb05e Compare September 17, 2024 08:11
@romayalon romayalon force-pushed the romy-check-access-configuration branch from 139631d to bb1f655 Compare September 17, 2024 16:38
shirady
shirady reviewed Sep 22, 2024
View reviewed changes
src/manage_nsfs/health.js Show resolved Hide resolved
src/util/native_fs_utils.js Show resolved Hide resolved
src/test/unit_tests/test_nc_nsfs_health.js Show resolved Hide resolved
shirady
shirady approved these changes Sep 23, 2024
View reviewed changes
Copy link
Contributor

@shirady shirady left a comment

Choose a reason for hiding this comment

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

LGTM

@romayalon
NC | Check access - skip write check and mode bits check by default a…
61bd3ee 
…dd health access check configuration

Signed-off-by: Romy <[email protected]>
@romayalon romayalon force-pushed the romy-check-access-configuration branch from 5833de6 to 61bd3ee Compare September 23, 2024 15:56
@romayalon romayalon merged commit 504f3b4 into noobaa:master Sep 24, 2024
10 checks passed
@naveenpaul1 naveenpaul1 mentioned this pull request Oct 18, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@naveenpaul1 naveenpaul1 naveenpaul1 left review comments

@shirady shirady shirady approved these changes

@guymguym guymguym Awaiting requested review from guymguym

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NSFS | NC | Update check access implementation to support flags
3 participants
@romayalon @shirady @naveenpaul1