-
Notifications
You must be signed in to change notification settings - Fork 27
Conversation
de38b63
to
0d1a335
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
OK so this doc only change is now hitting the same bits in #107 (comment) |
OK this should be fixed by https://gitlab.com/redhat/centos-stream/rpms/rust-bootupd/-/merge_requests/9 |
Not a blocker, would it make sense to provide code samples? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
(Even better, if this is for code executed as part of a systemd unit, investigate | ||
using `DynamicUser=yes`) | ||
|
||
However, `sysusers.d` only works for "system" users, not human login users. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this really true? 🤔 I can set a shell via sysusers.d and have a fully capable user (for kiosk for instance, but I think I can login too just fine)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well the man page has
This tool may be used to allocate system users and groups only, it is not useful for creating non-system (i.e. regular, "human") users and groups, as it accesses
/etc/passwd and /etc/group directly, bypassing any more complex user databases, for example any database involving NIS or LDAP.
But I think you're right it can be coaxed into doing the allocation part; there was also a discussion on fedora devel about extending this.
LGTM apart from a tiny question (non-blocking) |
docs/builds.md
Outdated
``` | ||
|
||
For SSH keys, one approach is to hardcode the SSH authorized keys under `/usr` so it's part of | ||
the clearly immutable state: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd stress this is something "easily" doable for the root user as the example shows - for "other" users, tying the containerfile to whichever way we actually add user at installation seems a bit off to me (as the users are not there, root is)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I just fear this could set a bad example going forward, and I'd leave ssh keys to the "provisioning/deploy" part)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many, if not most deployments will definitely want the flexibility of providing keys externally. But not all. And it's also convenient to have the keys part of the container image for those that want to be able to update/rotate it "day 2" without extra tooling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with that, I think it's just weird to add keys to an image for users other than root as there's nothing that says "and then create this user" so these keys are just laying around (no biggie tho, just something I remember also Timothee brought up when I demo'ed the kiosk app)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we point users to cloud-init and ignition?
Adding SSH works for some use cases but I feel like cloud-init and ignition offer much more flexibility.
0d1a335
to
13bd265
Compare
I just did cloud-init for now...ignition requires a lot of operating system integration and runs really fast into the case of "maybe I should be deriving from the edge/coreos image" which is its own world. |
Since this is a complex tricky topic.
13bd265
to
a586acb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Since this is a complex tricky topic.