Skip to content

Conversation

sigv
Copy link
Contributor

@sigv sigv commented May 19, 2023

Proposed changes

There is no use-case that is relevant for enforcing the fsGroup as the default user's group. Our nginx user will already have access to the volumes being created, as they are made with g+rwx.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@sigv sigv requested a review from a team as a code owner May 19, 2023 16:59
@github-actions github-actions bot added documentation Pull requests/issues for documentation helm_chart Pull requests that update the Helm Chart labels May 19, 2023
@sigv sigv mentioned this pull request May 19, 2023
6 tasks
@sigv sigv force-pushed the defaultFsGroup branch 2 times, most recently from 63b18a5 to 325ac4e Compare May 22, 2023 14:47
@codecov
Copy link

codecov bot commented May 23, 2023

Codecov Report

Merging #3926 (a57b092) into main (22c165b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #3926   +/-   ##
=======================================
  Coverage   51.81%   51.81%           
=======================================
  Files          59       59           
  Lines       16697    16697           
=======================================
  Hits         8651     8651           
  Misses       7747     7747           
  Partials      299      299           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sigv sigv force-pushed the defaultFsGroup branch 7 times, most recently from 2f3f5ec to c987520 Compare May 26, 2023 18:19
@sigv sigv force-pushed the defaultFsGroup branch 2 times, most recently from 6e5806b to e501948 Compare May 30, 2023 18:43
Copy link

@lucacome lucacome left a comment

Choose a reason for hiding this comment

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

Makes sense, we also set the group to 0 (root) for our files, so we don't need to enforce 101.

Thanks @sigv !

@lucacome lucacome requested a review from a team May 30, 2023 18:55
@sigv sigv force-pushed the defaultFsGroup branch from e501948 to ca132d9 Compare May 31, 2023 08:21
There is no use-case that is relevant for enforcing the `fsGroup`
as the default user's group. Our `nginx` user will already have
access to the volumes being created, as they are made with `g+rwx`.
@sigv sigv force-pushed the defaultFsGroup branch from ca132d9 to a57b092 Compare May 31, 2023 10:59
@lucacome lucacome merged commit ec1764b into nginx:main May 31, 2023
@sigv sigv deleted the defaultFsGroup branch June 1, 2023 06:44
@lucacome lucacome added chore Pull requests for routine tasks and removed helm_chart Pull requests that update the Helm Chart labels Jun 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Pull requests for routine tasks documentation Pull requests/issues for documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants