osbuild: add force option to org.osbuild.groups and always enable [HMS-9825] #2146
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
achilleas-k
force-pushed
the
stages/groups/force
branch
from
4df7153 to
ba4f47e
Compare
|
Cross-arch qemu based image build/boot smoke test will be fixed by #2145 |
| func TestNewGroupsStageOptions(t *testing.T) { | ||
| type testCase struct { | ||
| groups []users.Group | ||
| expOptions *GroupsStageOptions |
There was a problem hiding this comment.
I didn't initially parse this as meaning expected options :) I see we've used both expOptions and expectedOptions in various places so it might be nice to just use the long form.
thozza
force-pushed
the
stages/groups/force
branch
from
ba4f47e to
2d14832
Compare
bcl
left a comment
bcl
left a comment
There was a problem hiding this comment.
Looks good, I don't feel too strongly about the naming of expOptions :)
hah, funny you should mention it, I felt it was a bit too lazy to call it that, paused for a second, thought I shouldn't spend much time thinking hard about a variable name in a tiny test, and moved on. |
thozza
left a comment
thozza
left a comment
There was a problem hiding this comment.
In general, LGTM.
I've added a few comments. I think that the behavior of the --force option with two groups with the same GIDs is worth addressing, but could be in a follow up or as a documentation.
| // groups in their configs without worrying about the group existing in | ||
| // the base system or being created from a package, which isn't | ||
| // predictable. Enable it unconditionally for all builds | ||
| Force: true, |
There was a problem hiding this comment.
Nitpick: After reading the man page, it also When used with -g, and the specified GID already exists, another (unique) GID is chosen (i.e. -g is turned off).. This means that if a user specifies two different groups with the same GID in the BP, one of them will get a different GID and the build will pass. This behavior should be probably called out explicitly, or we should have an option validation to catch it and don't generate such manifest.
There was a problem hiding this comment.
You're right. We can't control for conflicts between the blueprint and the packages/system, but we should make things more predictable by keeping the blueprint sane at least and error on duplicates, even though it would technically work. I think that wouldn't be surprising or unusual behaviour.
That might be more appropriate for validation in the blueprint itself though.
The note I added to the blueprint reference covers the GID behaviour too, though maybe that's not entirely clear either: https://github.com/osbuild/osbuild.github.io/pull/188/changes
There was a problem hiding this comment.
ACK. Documentation and validation on the BP level sound good to me!
This version introduced the 'force' option in org.osbuild.groups. It's also the current latest release.
The --force option of the groupadd command makes it possible to run with a group name or GID that already exists without failing. This is very useful for letting users define groups in their configs without worrying about the group existing in the base system or being created from a package, which isn't always predictable.
The wheel group is created by the setup package on all systems by default. Before our use of the force option, this would fail. This new config now tests that the build doesn't fail since we enable the force option unconditionally.
achilleas-k
force-pushed
the
stages/groups/force
branch
from
2d14832 to
e5f415e
Compare
|
Here we go: osbuild/blueprint#43 |
5 participants
The
--forceoption of the groupadd command makes it possible to run with a group name or GID that already exists without failing. This is very useful for letting users define groups in their configs without worrying about the group existing in the base system or being created from a package, which isn't always predictable.