[gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 3337 bytes --]

Hello,

I think the policies proposed in GLEP 81 [1] were overenthusiastic
and they don't stand collision with sad Gentoo developer reality. 
Instead of improving the quality of resulting packages, they rather
hamper their adoption and cause growing frustration.

The problems I see today are:


1. Mailing list reviews hamper the adoption of new user packages.

Firstly, there are a few developers who obstructively refuse to
communicate with others and especially to use the public mailing lists. 
While this is a separate problem, and a problem that needs to be
resolved, this GLEP can't resolve it.  Of course, there is no reason to
believe that removing review requirement will actually make them migrate
their packages but it's at least one obstacle out of the way.

Secondly, even developers capable of communication find the two stage
request-wait-commit workflow inconvenient.  At any time, there are
at least a few requests waiting for being committed, possibly with
the developers forgetting about them.


2. Mailing list reviews don't serve their original purpose.

The original purpose of mailing list reviews was to verify that
the developers use new packages correctly.  For example, Michael
Orlitzky has found a lot of unnecessary home directories specified.
Of course, that works only if people submit *ebuilds* for review.

However, at some points developers arbitrarily decided to send only
numbers for review.  This defeats the purpose of the review in the first
place.


3. Cross-distro syncing has no purpose.

One of the original ideas was to reuse UID/GID numbers from other
distros when available to improve sync.  However, given the collisions
between old Gentoo UIDs and other distros, other distros themselves,
non-overlapping user/group names, etc. there seems to be little reason
to actually do it.  If we even managed some overlap, it would be minimal
and quasi-random.

While other distros provide a cheap way of choosing new UID/GID, it
doesn't seem that many people actually use it.  Then we hit pretty
absurd situations when someone chooses one UID/GID, somebody else tells
him to use the one from other distro.


4. Assignment mechanism is not collision-prone.

The secondary goal of mailing list reviews is to prevent UID/GID
collisions.  Sadly, it doesn't work there either.  Sometimes two people
request the same UID/GID, and only sometimes somebody else notices.
In the end, people have hard time figuring out which number is the 'next
free', sometimes they discover the number's been taken when somebody
else commits it first.


All that considered, I'd like to open discussion how we could improve
things.

My proposal would be to:

a. split the UID/GID range into 'high' (app) and 'low' (system)
assignments, 'high' being >=100 and 'low' <100 (matching Apache suEXEC
defaults),

b. UIDs/GIDs in the 'high' range can be taken arbitrarily (recommending
taking highest free), while in the 'low' range must be approved by QA,

c. no review requirement for the 'high' range, just choose your UID/GID
straight of uid-gid.txt and commit it,

d. strong recommendation to use matching UID/GID for the same user/group
name.

WDYT?


[1] https://www.gentoo.org/glep/glep-0081.html

-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Ulrich Mueller]

[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]

>>>>> On Mon, 09 Dec 2019, Michał Górny wrote:

> My proposal would be to:

> a. split the UID/GID range into 'high' (app) and 'low' (system)
> assignments, 'high' being >=100 and 'low' <100 (matching Apache suEXEC
> defaults),

Good, but can we make these ranges more explicit please, like 100..499
for "high" and 0..99 for "low"? (But 100 is special too, I guess?)

> b. UIDs/GIDs in the 'high' range can be taken arbitrarily
> (recommending taking highest free),

I'd say something like this:

"b. UIDs/GIDs in the 'high' range can be taken arbitrarily and are
assigned on a FCFS basis. IDs used upstream or by other distros can
serve as a loose guideline. Otherwise, taking the highest free number
in the range is recommended."

> while in the 'low' range must be approved by QA,

> c. no review requirement for the 'high' range, just choose your
> UID/GID straight of uid-gid.txt and commit it,

> d. strong recommendation to use matching UID/GID for the same
> user/group name.

Ulrich

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Ulrich Mueller]

[-- Attachment #1: Type: text/plain, Size: 442 bytes --]

>>>>> On Mon, 09 Dec 2019, Ulrich Mueller wrote:

>> a. split the UID/GID range into 'high' (app) and 'low' (system)
>> assignments, 'high' being >=100 and 'low' <100 (matching Apache suEXEC
>> defaults),

> Good, but can we make these ranges more explicit please, like 100..499
> for "high" and 0..99 for "low"? (But 100 is special too, I guess?)

I just see that the default /etc/login.defs has SYS_UID_MIN=101 and
SYS_GID_MIN=101.

Ulrich

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Thomas Deutschmann]

[-- Attachment #1.1: Type: text/plain, Size: 877 bytes --]

Hi,

I fully agree with #3. But I would go one step further:

Make complete SYS_UID_MIN - SYS_UID_MAX range available for GLEP 81.

The only argument/reason I am aware of, "but 501-999 could be used by
system" (Dynamic allocation by user.eclass), isn't strong enough from my
POV because system administrator can already pick something between
SYS_UID_MIN-MAX so system must be already capable of dealing with such
scenarios. So why do you think this range must be reserved? Isn't
blocking 501-999 just another random choice?

Therefor I would change the recommendation to pick highest free number.
I.e. it should be recommended to pick the lowest free UID/GID pair
instead (just to avoid fragmentation and keep 501+ free as long as
possible).


-- 
Regards,
Thomas Deutschmann / Gentoo Linux Developer
C4DD 695F A713 8F24 2AA1 5638 5849 7EE5 1D5D 74A5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Ulrich Mueller]

[-- Attachment #1: Type: text/plain, Size: 1238 bytes --]

>>>>> On Mon, 09 Dec 2019, Thomas Deutschmann wrote:

> Make complete SYS_UID_MIN - SYS_UID_MAX range available for GLEP 81.

> The only argument/reason I am aware of, "but 501-999 could be used by
> system" (Dynamic allocation by user.eclass), isn't strong enough from my
> POV because system administrator can already pick something between
> SYS_UID_MIN-MAX so system must be already capable of dealing with such
> scenarios. So why do you think this range must be reserved? Isn't
> blocking 501-999 just another random choice?

> Therefor I would change the recommendation to pick highest free number.
> I.e. it should be recommended to pick the lowest free UID/GID pair
> instead (just to avoid fragmentation and keep 501+ free as long as
> possible).

One problem with that is that at some time user.eclass dynamically
allocated IDs starting at 101 upwards, and later this was changed to 999
downwards (at different times for UIDs and GIDs). So for existing
systems you can expect some range above 101 and some range below 999 to
be occupied. So it may not be the best idea to start picking IDs from
101 upwards, which are most likely to collide.

If the range below 500 should fill up completely, we can always
reconsider.

Ulrich

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Thomas Deutschmann]

[-- Attachment #1.1: Type: text/plain, Size: 1147 bytes --]

On 2019-12-09 18:47, Ulrich Mueller wrote:
> One problem with that is that at some time user.eclass dynamically
> allocated IDs starting at 101 upwards, and later this was changed to 999
> downwards (at different times for UIDs and GIDs). So for existing
> systems you can expect some range above 101 and some range below 999 to
> be occupied. So it may not be the best idea to start picking IDs from
> 101 upwards, which are most likely to collide.

Sure, but what's the problem here? Or let me rephrase: Which problem do
you try to avoid/address with blocking 501-999 for now?

Like said, if an ID is already taken for any reason on user's system,
that's not a problem. acct-* can handle that... there's nothing like a
collision.

And until user.eclass is completely gone, all packages are migrated to
GLEP 81 and all users have completely reinstalled their Gentoo systems
(most packages used dynamic allocation until GLEP 81), you won't have
"clean", collision free systems with same ID all over the places.


-- 
Regards,
Thomas Deutschmann / Gentoo Linux Developer
C4DD 695F A713 8F24 2AA1 5638 5849 7EE5 1D5D 74A5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Ulrich Mueller]

[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]

>>>>> On Mon, 09 Dec 2019, Thomas Deutschmann wrote:

> Like said, if an ID is already taken for any reason on user's system,
> that's not a problem. acct-* can handle that... there's nothing like a
> collision.

You can call it "collision" or something else, the fact is that in this
scenario, the acct-* package won't get its preferred ID. Which is the
whole point of the migration to static IDs. You can consider this
unimportant, but why do we have GLEP 81 then, in the first place?

> And until user.eclass is completely gone, all packages are migrated to
> GLEP 81 and all users have completely reinstalled their Gentoo systems
> (most packages used dynamic allocation until GLEP 81), you won't have
> "clean", collision free systems with same ID all over the places.

Right now, new systems will have their dynamic IDs allocated from 999
downwards, so it is very unlikely that they will collide with the ones
that are statically allocated. However, they most certainly will if we
allow allocation in the upper range.

Also, what about users calling "useradd -r" manually, for whatever
purpose? They'll get IDs counting from 999 downwards as well, even after
the transition will be complete.

Ulrich

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Thomas Deutschmann]

[-- Attachment #1.1: Type: text/plain, Size: 3221 bytes --]

On 2019-12-09 19:48, Ulrich Mueller wrote:
>>>>>> On Mon, 09 Dec 2019, Thomas Deutschmann wrote:
> 
>> Like said, if an ID is already taken for any reason on user's system,
>> that's not a problem. acct-* can handle that... there's nothing like a
>> collision.
> 
> You can call it "collision" or something else, the fact is that in this
> scenario, the acct-* package won't get its preferred ID. Which is the
> whole point of the migration to static IDs. You can consider this
> unimportant, but why do we have GLEP 81 then, in the first place?

Heh, I am sorry but I think your expectation is wrong here:

From my understanding, the authors of GLEP 81 should correct me if I am
wrong, the main idea was to get stable IDs across multiple Gentoo systems.

I personally doubt that it's worth because it's very rare that you will
only have to deal with Gentoo boxes so if you really *need* same user/ID
for some reason you will have other mechanism already in place
(configuration management, LDAP..). Of course, if you only have to deal
with Gentoo, it might save you some work.

Anyway, now we have GLEP 81. However, everyone should be clear about the
fact that GLEP 81 migration *cannot* and will *not* work for *existing*
systems which have the packages before GLEP 81 became a thing already
installed.

That's because acct-* will *not* and *cannot* alter existing user.

In practice, nobody maintaining a lot of systems will actually reinstall
all their Gentoo boxes just to get these new IDs. Like said, if they
care about ids, they already have other mechanism in place.

So when we talk about GLEP 81 we *can* only talk about greenfield aka
new installation. No need to care about "collision" on systems before
GLEP 81 (you cannot avoid them and it's just not worth)...


tl;dr
GLEP 81 describes a perfect world scenario. It's not giving us anything
for real and anyone carrying about numbers must probably have mechanism
in place which will handle that and work not just on Gentoo.

That said, blocking 501-999 for now could be a valid goal to avoid
fragmentation and see how things will work. When we decide to do that,
we should document the exact reason. Saying "because of user.eclass or
to avoid collision" is not a valid reason.


> Also, what about users calling "useradd -r" manually, for whatever
> purpose? They'll get IDs counting from 999 downwards as well, even after
> the transition will be complete.

shadow does that to avoid reuse of ids used by former, now deleted
users, see
https://github.com/shadow-maint/shadow/blob/4.8/libmisc/find_new_uid.c#L224

It's just an attempt to be smart. It's assuming that system(packages)
will go upwards so when the user invoking useradd will go downwards you
will probably not reuse id from user which got deleted but is still
owning files/ACLs.

Note: That's why Debian's useradd wrapper, adduser, is doing the
opposite. It's starting with MIN and is going upwards... so we should do
the same when picking IDs to help shadow being smart and avoid reuse as
long as possible.


-- 
Regards,
Thomas Deutschmann / Gentoo Linux Developer
C4DD 695F A713 8F24 2AA1 5638 5849 7EE5 1D5D 74A5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Michael Orlitzky]

On 12/9/19 3:10 PM, Thomas Deutschmann wrote:
> On 2019-12-09 19:48, Ulrich Mueller wrote:
>>>>>>> On Mon, 09 Dec 2019, Thomas Deutschmann wrote:
>>
>>> Like said, if an ID is already taken for any reason on user's system,
>>> that's not a problem. acct-* can handle that... there's nothing like a
>>> collision.
>>
>> You can call it "collision" or something else, the fact is that in this
>> scenario, the acct-* package won't get its preferred ID. Which is the
>> whole point of the migration to static IDs. You can consider this
>> unimportant, but why do we have GLEP 81 then, in the first place?
> 
> Heh, I am sorry but I think your expectation is wrong here:
> 
> From my understanding, the authors of GLEP 81 should correct me if I am
> wrong, the main idea was to get stable IDs across multiple Gentoo systems.
> 

Since I have been summoned... the stable UID/GIDs were not the main
motivation for GLEP81. The big problem it solves is that we had multiple
packages creating "shared" users in the same namespace, either

  (a) without knowing that they were shared, or
  (b) trying to keep every piece of copy/pasted user data in sync.

This lead to some dumb security vulnerabilities, like using the "milter"
user for anything that looks at an email. On a mail server you wind up
with five unrelated daemons being able to write to each others' private
files. For another example, the "ntp" user was fought over by at least
two packages that insisted on certain (mutually exclusive) settings for
the same user. Whether or not your daemon worked depended on which
ebuild was re-emerged last.

GLEP81 solves that, and factors out the user data so that multiple
packages can share a user without having to copy/paste its settings and
keep them synchronized. It also addresses the fact that some users are
needed at both build/runtime, while others are needed only at runtime.
In the past, enewuser calls were placed randomly in one of pkg_setup()
or pkg_preinst(), and you'd wind up with users on your system for
packages that failed to build. Now you just depend on the acct-user
package in DEPEND or RDEPEND like anything else.

The very first RFC didn't even include fixed UIDs, but a lot of people
requested them. I can see how it's useful. Every few years, I have to
replace a mail server and rsync over the contents of the mail store. On
the new server, I have to be very careful that the UIDs match up, which
might not be the case depending on the order certain packages were
emerged in. It's not too hard to do, but you have to know to do it, and
it wastes some time. It'll be great not having to worry about that next
time. If developers can spend a few minutes picking out a fixed UID to
save users an hour here and there, I think it's worth it, because there
are a lot more users than developers.

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Alec Warner]

[-- Attachment #1: Type: text/plain, Size: 4007 bytes --]

On Mon, Dec 9, 2019 at 12:17 AM Michał Górny <mgorny@gentoo.org> wrote:

> Hello,
>
> I think the policies proposed in GLEP 81 [1] were overenthusiastic
> and they don't stand collision with sad Gentoo developer reality.
> Instead of improving the quality of resulting packages, they rather
> hamper their adoption and cause growing frustration.
>
> The problems I see today are:
>
>
> 1. Mailing list reviews hamper the adoption of new user packages.
>
> Firstly, there are a few developers who obstructively refuse to
> communicate with others and especially to use the public mailing lists.
> While this is a separate problem, and a problem that needs to be
> resolved, this GLEP can't resolve it.  Of course, there is no reason to
> believe that removing review requirement will actually make them migrate
> their packages but it's at least one obstacle out of the way.
>
> Secondly, even developers capable of communication find the two stage
> request-wait-commit workflow inconvenient.  At any time, there are
> at least a few requests waiting for being committed, possibly with
> the developers forgetting about them.
>
>
> 2. Mailing list reviews don't serve their original purpose.
>
> The original purpose of mailing list reviews was to verify that
> the developers use new packages correctly.  For example, Michael
> Orlitzky has found a lot of unnecessary home directories specified.
> Of course, that works only if people submit *ebuilds* for review.
>
> However, at some points developers arbitrarily decided to send only
> numbers for review.  This defeats the purpose of the review in the first
> place.
>
>
> 3. Cross-distro syncing has no purpose.
>
> One of the original ideas was to reuse UID/GID numbers from other
> distros when available to improve sync.  However, given the collisions
> between old Gentoo UIDs and other distros, other distros themselves,
> non-overlapping user/group names, etc. there seems to be little reason
> to actually do it.  If we even managed some overlap, it would be minimal
> and quasi-random.
>
> While other distros provide a cheap way of choosing new UID/GID, it
> doesn't seem that many people actually use it.  Then we hit pretty
> absurd situations when someone chooses one UID/GID, somebody else tells
> him to use the one from other distro.
>
>
> 4. Assignment mechanism is not collision-prone.
>
> The secondary goal of mailing list reviews is to prevent UID/GID
> collisions.  Sadly, it doesn't work there either.  Sometimes two people
> request the same UID/GID, and only sometimes somebody else notices.
> In the end, people have hard time figuring out which number is the 'next
> free', sometimes they discover the number's been taken when somebody
> else commits it first.
>
>
> All that considered, I'd like to open discussion how we could improve
> things.
>
> My proposal would be to:
>
> a. split the UID/GID range into 'high' (app) and 'low' (system)
> assignments, 'high' being >=100 and 'low' <100 (matching Apache suEXEC
> defaults),
>
> b. UIDs/GIDs in the 'high' range can be taken arbitrarily (recommending
> taking highest free), while in the 'low' range must be approved by QA,
>
> c. no review requirement for the 'high' range, just choose your UID/GID
> straight of uid-gid.txt and commit it.
>

What is the mechanism to keep the uid-gid.txt aligned with tree content? is
there a CI check that says I am using the new acct-* eclasses AND I have a
UID / GID assigned that is not matching uid-gid.txt? I see the CI has
"ConflictingAccountIdentifiers", is this already doing this work (checking
that the ebuild matchines uid-gid.txt), or just scanning the whole tree and
ensuring that 2 packages don't re-use the same ID?

-A


>
> d. strong recommendation to use matching UID/GID for the same user/group
> name.
>
> WDYT?
>
>
> [1] https://www.gentoo.org/glep/glep-0081.html
>
> --
> Best regards,
> Michał Górny
>
>

[-- Attachment #2: Type: text/html, Size: 4910 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 4101 bytes --]

On Mon, 2019-12-09 at 13:48 -0800, Alec Warner wrote:
> On Mon, Dec 9, 2019 at 12:17 AM Michał Górny <mgorny@gentoo.org> wrote:
> 
> > Hello,
> > 
> > I think the policies proposed in GLEP 81 [1] were overenthusiastic
> > and they don't stand collision with sad Gentoo developer reality.
> > Instead of improving the quality of resulting packages, they rather
> > hamper their adoption and cause growing frustration.
> > 
> > The problems I see today are:
> > 
> > 
> > 1. Mailing list reviews hamper the adoption of new user packages.
> > 
> > Firstly, there are a few developers who obstructively refuse to
> > communicate with others and especially to use the public mailing lists.
> > While this is a separate problem, and a problem that needs to be
> > resolved, this GLEP can't resolve it.  Of course, there is no reason to
> > believe that removing review requirement will actually make them migrate
> > their packages but it's at least one obstacle out of the way.
> > 
> > Secondly, even developers capable of communication find the two stage
> > request-wait-commit workflow inconvenient.  At any time, there are
> > at least a few requests waiting for being committed, possibly with
> > the developers forgetting about them.
> > 
> > 
> > 2. Mailing list reviews don't serve their original purpose.
> > 
> > The original purpose of mailing list reviews was to verify that
> > the developers use new packages correctly.  For example, Michael
> > Orlitzky has found a lot of unnecessary home directories specified.
> > Of course, that works only if people submit *ebuilds* for review.
> > 
> > However, at some points developers arbitrarily decided to send only
> > numbers for review.  This defeats the purpose of the review in the first
> > place.
> > 
> > 
> > 3. Cross-distro syncing has no purpose.
> > 
> > One of the original ideas was to reuse UID/GID numbers from other
> > distros when available to improve sync.  However, given the collisions
> > between old Gentoo UIDs and other distros, other distros themselves,
> > non-overlapping user/group names, etc. there seems to be little reason
> > to actually do it.  If we even managed some overlap, it would be minimal
> > and quasi-random.
> > 
> > While other distros provide a cheap way of choosing new UID/GID, it
> > doesn't seem that many people actually use it.  Then we hit pretty
> > absurd situations when someone chooses one UID/GID, somebody else tells
> > him to use the one from other distro.
> > 
> > 
> > 4. Assignment mechanism is not collision-prone.
> > 
> > The secondary goal of mailing list reviews is to prevent UID/GID
> > collisions.  Sadly, it doesn't work there either.  Sometimes two people
> > request the same UID/GID, and only sometimes somebody else notices.
> > In the end, people have hard time figuring out which number is the 'next
> > free', sometimes they discover the number's been taken when somebody
> > else commits it first.
> > 
> > 
> > All that considered, I'd like to open discussion how we could improve
> > things.
> > 
> > My proposal would be to:
> > 
> > a. split the UID/GID range into 'high' (app) and 'low' (system)
> > assignments, 'high' being >=100 and 'low' <100 (matching Apache suEXEC
> > defaults),
> > 
> > b. UIDs/GIDs in the 'high' range can be taken arbitrarily (recommending
> > taking highest free), while in the 'low' range must be approved by QA,
> > 
> > c. no review requirement for the 'high' range, just choose your UID/GID
> > straight of uid-gid.txt and commit it.
> > 
> 
> What is the mechanism to keep the uid-gid.txt aligned with tree content? is
> there a CI check that says I am using the new acct-* eclasses AND I have a
> UID / GID assigned that is not matching uid-gid.txt? I see the CI has
> "ConflictingAccountIdentifiers", is this already doing this work (checking
> that the ebuild matchines uid-gid.txt), or just scanning the whole tree and
> ensuring that 2 packages don't re-use the same ID?
> 

The latter.

-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Joonas Niilola]

[-- Attachment #1.1: Type: text/plain, Size: 2999 bytes --]

Hey,

On 12/9/19 10:17 AM, Michał Górny wrote:
> Hello,
>
> I think the policies proposed in GLEP 81 [1] were overenthusiastic
> and they don't stand collision with sad Gentoo developer reality. 
> Instead of improving the quality of resulting packages, they rather
> hamper their adoption and cause growing frustration.
>
> The problems I see today are:
>
>
> 2. Mailing list reviews don't serve their original purpose.
>
> The original purpose of mailing list reviews was to verify that
> the developers use new packages correctly.  For example, Michael
> Orlitzky has found a lot of unnecessary home directories specified.
> Of course, that works only if people submit *ebuilds* for review.
>
> However, at some points developers arbitrarily decided to send only
> numbers for review.  This defeats the purpose of the review in the first
> place.

The problem: There is still no any official documentation about using
acct-, and reviewing it was/is pretty much left on the shoulders of one
man. It's easy to say on hindsight it was implemented too quickly.


>
>
> 4. Assignment mechanism is not collision-prone.
>
> The secondary goal of mailing list reviews is to prevent UID/GID
> collisions.  Sadly, it doesn't work there either.  Sometimes two people
> request the same UID/GID, and only sometimes somebody else notices.
> In the end, people have hard time figuring out which number is the 'next
> free', sometimes they discover the number's been taken when somebody
> else commits it first.

If I remember correctly, at one point it was agreed not to paste ebuilds
because they all just looked similar, but just ask for IDs?


>
>
> All that considered, I'd like to open discussion how we could improve
> things.
>
> My proposal would be to:
>
> a. split the UID/GID range into 'high' (app) and 'low' (system)
> assignments, 'high' being >=100 and 'low' <100 (matching Apache suEXEC
> defaults),
>
> b. UIDs/GIDs in the 'high' range can be taken arbitrarily (recommending
> taking highest free), while in the 'low' range must be approved by QA,
>
> c. no review requirement for the 'high' range, just choose your UID/GID
> straight of uid-gid.txt and commit it,
>
> d. strong recommendation to use matching UID/GID for the same user/group
> name.
>
> WDYT?
>
>
> [1] https://www.gentoo.org/glep/glep-0081.html


I think none of the above really prevent collisions for unmotivated
people. They also still require manual update of uid-gid.txt, and it
can't be expected everyone does it. Now this is not of a big interest to
devs, but I believe committing non-dev acct's will get hard here,
because there might be some "lag" with their contributions vrt. the
current situation.


Honestly I'd say just put -1 on all acct- packages then let sys admins
modify them locally to whatever they need. I feel like this whole GLEP
just serves the minority while making many other contributors uneasy.

-- juippis



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 642 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Rich Freeman]

On Tue, Dec 10, 2019 at 12:44 AM Joonas Niilola <juippis@gentoo.org> wrote:
>
> Honestly I'd say just put -1 on all acct- packages then let sys admins
> modify them locally to whatever they need. I feel like this whole GLEP
> just serves the minority while making many other contributors uneasy.
>

I think we're worrying far too much about people with decade-old
installs.  Just come up with a reasonable set of defaults and as long
as it can adapt to whatever is already in /etc/passwd we're fine.

Having UIDs chosen completely at random seems fairly non-optimal.
Suppose you're building containers/etc and then bind-mounting in
persistent storage (/var/lib/mysql and so on).  Wouldn't it be nice if
the default were that mysql would get the same UID on every build?  I
guess you could provide an initial /etc/passwd on every fresh build
but it just seems like an extra step.

This isn't about serving the minority so much as not letting the
perfect be the enemy of the good.  Yes, there are reasons why GLEP 81
won't be perfect.  That doesn't mean that it isn't a good idea...

-- 
Rich

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Thomas Deutschmann]

[-- Attachment #1.1: Type: text/plain, Size: 1501 bytes --]

Hi,

On 2019-12-10 12:47, Rich Freeman wrote:
> Having UIDs chosen completely at random seems fairly non-optimal.
> Suppose you're building containers/etc and then bind-mounting in
> persistent storage (/var/lib/mysql and so on).  Wouldn't it be nice if
> the default were that mysql would get the same UID on every build?  I
> guess you could provide an initial /etc/passwd on every fresh build
> but it just seems like an extra step.

While this sounds like a valid problem we are going to address, this
sounds like an analysis without practical experience:

In practice you will *never* assume proper container <> host user
mapping. *Never*. If you do that, you are doing it wrong:

- Container sometimes switch base images. You won't notice that unless
you follow container provider very closely. But you are using container
because you are focused on containerized application, not the container
itself...

- Container start doing things differently. Again, you won't notice, see
above.

- Your host is maybe running some real services. You really don't want
that a container suddenly become able to access these services just
because container <> host mapping has match.

No, when you follow best practice you will always pass user/group or use
other available mapping solutions.

So while it sounds like a valid *goal*, in real world, it isn't.


-- 
Regards,
Thomas Deutschmann / Gentoo Linux Developer
C4DD 695F A713 8F24 2AA1 5638 5849 7EE5 1D5D 74A5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Rich Freeman]

On Tue, Dec 10, 2019 at 7:26 AM Thomas Deutschmann <whissi@gentoo.org> wrote:
>
> On 2019-12-10 12:47, Rich Freeman wrote:
> > Having UIDs chosen completely at random seems fairly non-optimal.
> > Suppose you're building containers/etc and then bind-mounting in
> > persistent storage (/var/lib/mysql and so on).  Wouldn't it be nice if
> > the default were that mysql would get the same UID on every build?  I
> > guess you could provide an initial /etc/passwd on every fresh build
> > but it just seems like an extra step.
>
> In practice you will *never* assume proper container <> host user
> mapping. *Never*. If you do that, you are doing it wrong:

I'm not talking about container-host mapping.  I'm talking about
building the same container 100 times and having the container end up
with the same UIDs inside each time.

Build order in portage isn't really deterministic, especially over
long periods of time, so you can't rely on stuff getting installed in
the same order.

> - Container sometimes switch base images. You won't notice that unless
> you follow container provider very closely. But you are using container
> because you are focused on containerized application, not the container
> itself...

I'm talking about Gentoo containers here that YOU are the one
building.  Not just doing "docker run foo."  Obviously if you're using
somebody else's images you're going to end up with whatever UIDs they
use.  Chances are they're from some distro that actually DOES manage
their UIDs so they'll still be stable over time unless the base image
changes as you say.

> - Your host is maybe running some real services. You really don't want
> that a container suddenly become able to access these services just
> because container <> host mapping has match.

Uh, the container processes shouldn't even see the host
processes/files whether they have the same UIDs or not...

-- 
Rich

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Thomas Deutschmann]

[-- Attachment #1.1: Type: text/plain, Size: 1854 bytes --]

On 2019-12-10 13:44, Rich Freeman wrote:
> I'm not talking about container-host mapping.  I'm talking about
> building the same container 100 times and having the container end up
> with the same UIDs inside each time.
> 
> Build order in portage isn't really deterministic, especially over
> long periods of time, so you can't rely on stuff getting installed in
> the same order.

While I agree that portage doesn't guarantee you
deterministic/reproducible builds, in practice this isn't a problem:

Assume you are building a container for dev-db/mysql. I can only think
of one scenario where you would end up with different UIDs: That's when
dev-db/mysql (or a dependency) would suddenly create an own user and
will be merged before mysql's user was created.

But this is very theoretically. Especially in a container world, you
will create one container per services so it's *very* unlikely that
something like that will ever happen. Not?

Aside benefits from reproducible builds in general (which Gentoo doesn't
provide), please share reasons why one would care about used UIDs/GIDs
in containers...


> Uh, the container processes shouldn't even see the host
> processes/files whether they have the same UIDs or not...

Especially when you put mysql or any other service using data into a
container, service running in that container must be able to access this
data. And one common way to do that is allowing container to access data
stored on host, i.e.

> $ docker run \
>     --name some-mysql \
>     -v /my/own/datadir:/var/lib/mysql \
>     -e MYSQL_ROOT_PASSWORD=my-secret-pw \
>     -d mysql:tag

which will make /my/own/datadir from host available in container as
/var/lib/mysql.


-- 
Regards,
Thomas Deutschmann / Gentoo Linux Developer
C4DD 695F A713 8F24 2AA1 5638 5849 7EE5 1D5D 74A5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Rich Freeman]

On Tue, Dec 10, 2019 at 8:25 AM Thomas Deutschmann <whissi@gentoo.org> wrote:
>
> On 2019-12-10 13:44, Rich Freeman wrote:
> > I'm not talking about container-host mapping.  I'm talking about
> > building the same container 100 times and having the container end up
> > with the same UIDs inside each time.
> >
> > Build order in portage isn't really deterministic, especially over
> > long periods of time, so you can't rely on stuff getting installed in
> > the same order.
>
> Assume you are building a container for dev-db/mysql. I can only think
> of one scenario where you would end up with different UIDs: That's when
> dev-db/mysql (or a dependency) would suddenly create an own user and
> will be merged before mysql's user was created.
>
> But this is very theoretically. Especially in a container world, you
> will create one container per services so it's *very* unlikely that
> something like that will ever happen. Not?

There are cases where you might have more than one service in a
container, and there is certainly the issue of dependencies.  It
certainly makes sense to limit a container to a single function, but
internally that could involve multiple processes.

> Aside benefits from reproducible builds in general (which Gentoo doesn't
> provide), please share reasons why one would care about used UIDs/GIDs
> in containers...

Well, that is simple.  In the mysql example /var/lib/mysql would be
bind-mounted from outside the container, since it needs to be
persistent anytime the container is updated.  If you update the
container and it ends up with a different UID for mysqld, then it
wouldn't be able to read anything in /var/lib/mysql as it would still
have the previous UID.  You'd need to keep the two in sync somehow.

In fact, that is the very example you go on to offer...

> > Uh, the container processes shouldn't even see the host
> > processes/files whether they have the same UIDs or not...
>
> Especially when you put mysql or any other service using data into a
> container, service running in that container must be able to access this
> data. And one common way to do that is allowing container to access data
> stored on host, i.e.
>
> > $ docker run \
> >     --name some-mysql \
> >     -v /my/own/datadir:/var/lib/mysql \
> >     -e MYSQL_ROOT_PASSWORD=my-secret-pw \
> >     -d mysql:tag
>
> which will make /my/own/datadir from host available in container as
> /var/lib/mysql.
>

This is obviously exactly how you'd do it if you were using docker,
but you don't need to keep the UID in the container in sync with
anything else in the host.  If security is a concern you'd probably
want to make sure that nothing non-root can reach the directory since
its UID might be in use for something else.

In any case, this is why consistent UIDs in scratch installs are
useful.  When you're building a new version of a container you don't
want all its UIDs to change.  And of course this isn't just limited to
containers, but anything where you have persistent storage.  It is
just that the idea of creating new instances from scratch instead of
updating them in-place has become more fashionable around the same
time that containers have become more fashionable.  You could do the
same thing with a bare-metal host, though it would be a bit more
painful (perhaps using A/B partition booting, or less painful if
you're booting from a SAN or something like that).

-- 
Rich

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Joonas Niilola]

[-- Attachment #1.1: Type: text/plain, Size: 762 bytes --]


On 12/10/19 1:47 PM, Rich Freeman wrote:
>
> Having UIDs chosen completely at random seems fairly non-optimal.
> Suppose you're building containers/etc and then bind-mounting in
> persistent storage (/var/lib/mysql and so on).  Wouldn't it be nice if
> the default were that mysql would get the same UID on every build?  I
> guess you could provide an initial /etc/passwd on every fresh build
> but it just seems like an extra step.

I was more thinking along sys admins being able to modify their acct-
ebuilds with static numbers. If you're bind-mounting already, why not
bind your portage (or local overlay) to children as well. 2 minute more
work for those who need it, but a lot easier to everyone else who don't
care :)


-- juippis



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 642 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Michael Orlitzky]

On 12/10/19 11:05 AM, Joonas Niilola wrote:
> 
> I was more thinking along sys admins being able to modify their acct-
> ebuilds with static numbers. If you're bind-mounting already, why not
> bind your portage (or local overlay) to children as well. 2 minute more
> work for those who need it, but a lot easier to everyone else who don't
> care :)
> 

For most people, it's more convenient if the users/groups have the same
IDs on every system, but they don't actually care what those IDs are.
That's why it is the way it is, where developers pick basically any ID,
write it down, and hard-code it in the ebuild.

(Cross-distro compatibility is a stretch, but if we can make it work
easily in some cases then I don't see any harm in trying.)

If you need a specific ID, then by design you can make a new revision of
the ebuild in an overlay and tell the eclass to enforce your special ID.
But what we don't want is to force *every* user to create his own
overlay with *every* acct- ebuild just to get the same IDs on two
machines, since that's the sensible thing to do in the first place.

In any case, the collisions aren't why I supported mailing list review.
Users and groups are the most fundamental concept in UNIX security, and
the review requirement just reflects my belief that we can take a day or
two to make sure that we get them right.

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 3544 bytes --]

On Tue, 2019-12-10 at 07:44 +0200, Joonas Niilola wrote:
> Hey,
> 
> On 12/9/19 10:17 AM, Michał Górny wrote:
> > Hello,
> > 
> > I think the policies proposed in GLEP 81 [1] were overenthusiastic
> > and they don't stand collision with sad Gentoo developer reality. 
> > Instead of improving the quality of resulting packages, they rather
> > hamper their adoption and cause growing frustration.
> > 
> > The problems I see today are:
> > 
> > 
> > 2. Mailing list reviews don't serve their original purpose.
> > 
> > The original purpose of mailing list reviews was to verify that
> > the developers use new packages correctly.  For example, Michael
> > Orlitzky has found a lot of unnecessary home directories specified.
> > Of course, that works only if people submit *ebuilds* for review.
> > 
> > However, at some points developers arbitrarily decided to send only
> > numbers for review.  This defeats the purpose of the review in the first
> > place.
> 
> The problem: There is still no any official documentation about using
> acct-, and reviewing it was/is pretty much left on the shoulders of one
> man. It's easy to say on hindsight it was implemented too quickly.

There is official documentation in devmanual [1].

> > 
> > 4. Assignment mechanism is not collision-prone.
> > 
> > The secondary goal of mailing list reviews is to prevent UID/GID
> > collisions.  Sadly, it doesn't work there either.  Sometimes two people
> > request the same UID/GID, and only sometimes somebody else notices.
> > In the end, people have hard time figuring out which number is the 'next
> > free', sometimes they discover the number's been taken when somebody
> > else commits it first.
> 
> If I remember correctly, at one point it was agreed not to paste ebuilds
> because they all just looked similar, but just ask for IDs?

I wouldn't call it 'agreed'.  Someone said something, people stopped
doing.  Nobody bothered updating the policy (in GLEP 81, the rationale
explains it [2]).

> > All that considered, I'd like to open discussion how we could improve
> > things.
> > 
> > My proposal would be to:
> > 
> > a. split the UID/GID range into 'high' (app) and 'low' (system)
> > assignments, 'high' being >=100 and 'low' <100 (matching Apache suEXEC
> > defaults),
> > 
> > b. UIDs/GIDs in the 'high' range can be taken arbitrarily (recommending
> > taking highest free), while in the 'low' range must be approved by QA,
> > 
> > c. no review requirement for the 'high' range, just choose your UID/GID
> > straight of uid-gid.txt and commit it,
> > 
> > d. strong recommendation to use matching UID/GID for the same user/group
> > name.
> > 
> > WDYT?
> > 
> > 
> > [1] https://www.gentoo.org/glep/glep-0081.html
> 
> I think none of the above really prevent collisions for unmotivated
> people. They also still require manual update of uid-gid.txt, and it
> can't be expected everyone does it. Now this is not of a big interest to
> devs, but I believe committing non-dev acct's will get hard here,
> because there might be some "lag" with their contributions vrt. the
> current situation.
> 

Hence my idea that if we stop requiring mailing list RFC, we can replace
that with obligatory update to uid-gid.txt.  It should work good enough
for synchronization.


[1] https://devmanual.gentoo.org/ebuild-writing/users-and-groups/index.html
[2] https://www.gentoo.org/glep/glep-0081.html#requiring-mailing-list-rfc

-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Joonas Niilola]

[-- Attachment #1.1: Type: text/plain, Size: 1250 bytes --]


On 12/10/19 3:34 PM, Michał Górny wrote:
>> The problem: There is still no any official documentation about using
>> acct-, and reviewing it was/is pretty much left on the shoulders of one
>> man. It's easy to say on hindsight it was implemented too quickly.
> There is official documentation in devmanual [1].

The _detailed_ one was pushed 2 hours before I made my post, if that's
what you're referencing now.

https://gitweb.gentoo.org/proj/devmanual.git/commit/?id=9613e9e69ae16e6981f90135f92811ded641b52c

How could I have missed it? But yes, it's exactly and finally what was
needed for a long time.


>
> Hence my idea that if we stop requiring mailing list RFC, we can replace
> that with obligatory update to uid-gid.txt.  It should work good enough
> for synchronization.

Mmm, yeah sure, I guess it works better for everyone. I can still
imagine someone pushing acct- ebuilds with colliding UIDs, but at least
the CI checks for duplicates right? So committer should receive a mail
to change their numbers ASAP right? While at least with mailing list RFC
there's a small chance it can be prevented (like was done twice last
week), but the process is indeed more annoying and more manual.


-- juippis




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 642 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1783 bytes --]

On Tue, 2019-12-10 at 18:13 +0200, Joonas Niilola wrote:
> On 12/10/19 3:34 PM, Michał Górny wrote:
> > > The problem: There is still no any official documentation about using
> > > acct-, and reviewing it was/is pretty much left on the shoulders of one
> > > man. It's easy to say on hindsight it was implemented too quickly.
> > There is official documentation in devmanual [1].
> 
> The _detailed_ one was pushed 2 hours before I made my post, if that's
> what you're referencing now.
> 
> https://gitweb.gentoo.org/proj/devmanual.git/commit/?id=9613e9e69ae16e6981f90135f92811ded641b52c
> 
> How could I have missed it? But yes, it's exactly and finally what was
> needed for a long time.

No, I was referring to the short one.  But yes, this one is better.

> 
> 
> > Hence my idea that if we stop requiring mailing list RFC, we can replace
> > that with obligatory update to uid-gid.txt.  It should work good enough
> > for synchronization.
> 
> Mmm, yeah sure, I guess it works better for everyone. I can still
> imagine someone pushing acct- ebuilds with colliding UIDs, but at least
> the CI checks for duplicates right? So committer should receive a mail
> to change their numbers ASAP right?

Yes.

>  While at least with mailing list RFC
> there's a small chance it can be prevented (like was done twice last
> week), but the process is indeed more annoying and more manual.
> 

I wouldn't really call it 'prevented'.  Sure, somebody caught it
in time, and the other party noticed it in time.  However, if we remove
the waiting period related to reviews, the risk of collision is much
smaller.

That said, I need to add some pkgchecks for missync between uid-gid.txt
and acct-* packages.

-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Michael Orlitzky]

On 12/9/19 3:17 AM, Michał Górny wrote:
> 
> 2. Mailing list reviews don't serve their original purpose.
> 
> The original purpose of mailing list reviews was to verify that
> the developers use new packages correctly.  For example, Michael
> Orlitzky has found a lot of unnecessary home directories specified.
> Of course, that works only if people submit *ebuilds* for review.
> 
> However, at some points developers arbitrarily decided to send only
> numbers for review.  This defeats the purpose of the review in the first
> place.
> 

For users like "git" and "ntp" that are shared by multiple packages, I
think the mailing list review is still helpful. We fixed big problems
with those two that only came to light during review. If somebody else
requests a UID for "your" user, that gives you a chance to review their
acct-user ebuild and make sure that it's compatible with your package.

For esoteric packages with a dedicated user, though, you're probably
right. The main benefit of the mailing list posts so far is that they
let me track down pull requests and suggest that people ignore the
example in the devmanual.

If we can remove

  ACCT_USER_SHELL=/usr/bin/foo
  ACCT_USER_HOME=/var/lib/foo
  ACCT_USER_HOME_OWNER=foo:bar
  ACCT_USER_HOME_PERMS=0775

from [1], that would really help people guess the right thing to do the
first time around.


[1] https://devmanual.gentoo.org/ebuild-writing/users-and-groups/index.html

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1692 bytes --]

On Tue, 2019-12-10 at 09:50 -0500, Michael Orlitzky wrote:
> On 12/9/19 3:17 AM, Michał Górny wrote:
> > 2. Mailing list reviews don't serve their original purpose.
> > 
> > The original purpose of mailing list reviews was to verify that
> > the developers use new packages correctly.  For example, Michael
> > Orlitzky has found a lot of unnecessary home directories specified.
> > Of course, that works only if people submit *ebuilds* for review.
> > 
> > However, at some points developers arbitrarily decided to send only
> > numbers for review.  This defeats the purpose of the review in the first
> > place.
> > 
> 
> For users like "git" and "ntp" that are shared by multiple packages, I
> think the mailing list review is still helpful. We fixed big problems
> with those two that only came to light during review. If somebody else
> requests a UID for "your" user, that gives you a chance to review their
> acct-user ebuild and make sure that it's compatible with your package.
> 
> For esoteric packages with a dedicated user, though, you're probably
> right. The main benefit of the mailing list posts so far is that they
> let me track down pull requests and suggest that people ignore the
> example in the devmanual.
> 
> If we can remove
> 
>   ACCT_USER_SHELL=/usr/bin/foo
>   ACCT_USER_HOME=/var/lib/foo
>   ACCT_USER_HOME_OWNER=foo:bar
>   ACCT_USER_HOME_PERMS=0775
> 
> from [1], that would really help people guess the right thing to do the
> first time around.
> 

Feel free to submit a PR for that if you didn't include it in your big
changes (or make it separate to make it happen faster).

-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [gentoo-dev] [RFC] Revisiting GLEP 81 (acct-*) policies (reviews, cross-distro syncing)

[Rich Freeman]

On Tue, Dec 10, 2019 at 9:50 AM Michael Orlitzky <mjo@gentoo.org> wrote:
>
> For esoteric packages with a dedicated user, though, you're probably
> right. The main benefit of the mailing list posts so far is that they
> let me track down pull requests and suggest that people ignore the
> example in the devmanual.
>

Do the list reviews really put people off that much?  It seems like
eclasses.  Plenty of packages have one-off eclasses that nobody cares
about except the specific project, in which case the list posts are
just a formality and largely a NOOP.  However, this list isn't really
high-traffic.  Ditto with last-rites and so on.  I think having the
opportunity for review is probably worth it even if often it is just a
NOOP.

If people are afraid to post something for review because of potential
criticism then maybe we need to work more to make sure people
understand that everybody makes mistakes and nobody knows everything,
and this is why we have reviews in the first place.  Nobody is going
to have their commit access removed because they didn't notice
something and were thoughtful enough to get more eyes on it before
commiting it.  IMO that is a sign of responsible commit access.

-- 
Rich