Skip to content
This repository has been archived by the owner on Sep 18, 2020. It is now read-only.

Added fleet user and group and polkit rule #1579

Merged
merged 1 commit into from
Mar 15, 2016

Conversation

kayrus
Copy link
Contributor

@kayrus kayrus commented Oct 10, 2015

@marineam
Copy link
Contributor

Needs to bump the revision of the fleet ebuild. once the baselayout change is merged also add the bump of the baselayout ebuild to this PR.

@kayrus
Copy link
Contributor Author

kayrus commented Oct 12, 2015

@marineam Bumped revision to r2 and added policy rule mentioned in coreos/baselayout#38

@kayrus kayrus changed the title Added fleet user and group Added fleet user and group and polkit rule Oct 12, 2015
@@ -0,0 +1,6 @@
polkit.addRule(function(action, subject) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this makes more sense to install with fleet.

Copy link
Contributor

Choose a reason for hiding this comment

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

For clarity, from a distro packager's point of view the ideal package just ships upstream code and doesn't need distro specific hacks/patches. So given that fleet depends on systemd but not the other way around fleet related policy would be weird for systemd itself to ship.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved into fleet ebuild directory

@kayrus
Copy link
Contributor Author

kayrus commented Oct 13, 2015

Added tmpfiles fleet.conf and moved polkit rule into fleet ebuild directory.

@mischief
Copy link
Contributor

@kayrus if fleet does not run as root, how can it write into /run?

@kayrus
Copy link
Contributor Author

kayrus commented Oct 22, 2015

@mischief because I've added tmpfiles.d:

d    /var/run/fleet 0755 fleet fleet - -

kayrus added a commit to endocode/fleet that referenced this pull request Feb 5, 2016
@coreosbot
Copy link

Can one of the admins verify this patch?

@kayrus kayrus force-pushed the kayrus/fleet_group branch 2 times, most recently from 63cd3c4 to 5b42aee Compare February 29, 2016 10:05
kayrus added a commit to endocode/fleet that referenced this pull request Feb 29, 2016
kayrus added a commit to endocode/fleet that referenced this pull request Feb 29, 2016
kayrus added a commit to endocode/fleet that referenced this pull request Feb 29, 2016
kayrus added a commit to endocode/fleet that referenced this pull request Feb 29, 2016
kayrus added a commit to endocode/fleet that referenced this pull request Feb 29, 2016
@jonboulle
Copy link
Contributor

LGTM

@@ -0,0 +1,4 @@
# create fleet group
g fleet 253 - -
Copy link

Choose a reason for hiding this comment

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

@marineam are we intentionally allocating 253 to the fleet group in coreos?

Copy link
Contributor

Choose a reason for hiding this comment

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

253 is available, although since the ebuild doesn't set the ownership of anything or run tmpfiles in advance I think we can set it to - and let systemd allocate the id on first boot. Static ids should only be needed when maintaining compatibility with the old scheme in baselayout or the pre-built filesystems have ownership set for some reason.

Copy link
Contributor

Choose a reason for hiding this comment

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

s/^g/u/ as well, you need both a user and a group, not just a group.

@vcaputo
Copy link

vcaputo commented Mar 4, 2016

Just one question about the magic number used for the fleet group, generally looks good to me, not that I'm familiar with polkit rules.

@@ -4,4 +4,6 @@ PartOf=fleet.service

[Socket]
ListenStream=/var/run/fleet.sock

SocketMode=0660
SocketUser=fleet
Copy link
Contributor

Choose a reason for hiding this comment

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

sysusers isn't creating a fleet user, should it?

Copy link
Contributor

Choose a reason for hiding this comment

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

and I see, yes, it should

@kayrus
Copy link
Contributor Author

kayrus commented Mar 8, 2016

@marineam fixed

@kayrus
Copy link
Contributor Author

kayrus commented Mar 15, 2016

@marineam ping

@@ -0,0 +1,4 @@
# create fleet user and group
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this into files/sysusers.d/fleet.conf?

@@ -0,0 +1 @@
d /var/run/fleet 0750 fleet fleet - -
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this into files/tmpfiles.d/fleet.conf?

@crawford
Copy link
Contributor

Two more suggestions, otherwise LGTM.

@marineam
Copy link
Contributor

LGTM

@kayrus
Copy link
Contributor Author

kayrus commented Mar 15, 2016

@crawford @marineam done

crawford added a commit that referenced this pull request Mar 15, 2016
app-admin/fleet: add fleet user, group, and polkit rules
@crawford crawford merged commit 4752333 into coreos:master Mar 15, 2016
kayrus added a commit to endocode/fleet that referenced this pull request Mar 31, 2016
kayrus added a commit to endocode/fleet that referenced this pull request Mar 31, 2016
hectorj2f pushed a commit to giantswarm/fleet that referenced this pull request Apr 6, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants