Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

install: Add support for --root-ssh-authorized-keys #296

Merged
merged 2 commits into from
Feb 5, 2024

Conversation

cgwalters
Copy link
Collaborator

install: Rename clashing variable

I will need to access the outer state here in a later PR.


install: Add support for --root-ssh-authorized-keys

The current bootc install model is VERY opinionated: we install
the running container image to disk, and that is (almost!) it.

The only non-container out of band state that we support injecting right now
is kargs (via --karg) - we know we need this for machine local
kernel arguments.

(We do have a current outstanding PR to add a highly generic mechanism
to inject arbitrary files in /etc, but I want to think about that more)

THis current strict stance is quite painful for
a use case like "take a generic container image and bootc install to-filesystem --alongside"
in a cloud environment, because the generic container may not
have cloud-init.

With this change it becomes extremely convenient to:

  • Boot generic cloud image (e.g. AMI with apt/dnf + cloud-init)
  • cloud-init fetches SSH keys from hypervisor (instance metadata)
  • podman run -v /root/.ssh/authorized_keys:/keys:ro bootc install ... --root-ssh-authorized-keys=/keys`

And then the instance will carry forward those hypervisor-provided
keys but without a dependency on cloud-init.

Another use case for this of course is being the backend of things
like Anaconda's kickstart verbs which support injecting SSH keys.

Signed-off-by: Colin Walters walters@verbum.org


Copy link

openshift-ci bot commented Feb 2, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@github-actions github-actions bot added the area/install Issues related to `bootc install` label Feb 2, 2024
I will need to access the outer `state` here in a later PR.

Signed-off-by: Colin Walters <walters@verbum.org>
@cgwalters cgwalters force-pushed the install-inject-ssh branch 3 times, most recently from cdf4be7 to 51e246f Compare February 2, 2024 18:41
@cgwalters cgwalters marked this pull request as ready for review February 2, 2024 18:41
The current `bootc install` model is VERY opinionated: we install
the running container image to disk, and that is (almost!) it.

The only non-container out of band state that we support injecting right now
is kargs (via `--karg`) - we know we need this for machine local
kernel arguments.

(We do have a current outstanding PR to add a highly generic mechanism
 to inject arbitrary files in `/etc`, but I want to think about that more)

THis current strict stance is quite painful for
a use case like "take a generic container image and bootc install to-filesystem --alongside"
in a cloud environment, because the generic container may not
have cloud-init.

With this change it becomes extremely convenient to:

- Boot generic cloud image (e.g. AMI with apt/dnf + cloud-init)
- cloud-init fetches SSH keys from hypervisor (instance metadata)
- podman run -v /root/.ssh/authorized_keys:/keys:ro <image> bootc install ... --root-ssh-authorized-keys=/keys`

And then the instance will carry forward those hypervisor-provided
keys but without a dependency on cloud-init.

Another use case for this of course is being the backend of things
like Anaconda's kickstart verbs which support injecting SSH keys.

Signed-off-by: Colin Walters <walters@verbum.org>
@cgwalters
Copy link
Collaborator Author

OK now tested and working!

@cgwalters
Copy link
Collaborator Author

cgwalters commented Feb 2, 2024

There is an argument that we should support something like a more declarative full-blown user/ssh configuration. Some higher level projects do that (e.g. kairos). There's of course also Ignition etc.

I am hesitant a bit, but (a bit like install configuration) I think we could perhaps ship a bare minimum while avoiding scope creep here potentially. (edit: and that's what IMO this is: the bare minimum here is logging in as root via SSH. Passwords are IMO just a really really bad idea in general. Non-root users...not in the critical path, there are other ways to do that)

That said, such a thing would have immediate tension with user configuration as part of container builds. It's a messy topic.

Copy link
Member

@jmarrero jmarrero left a comment

Choose a reason for hiding this comment

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

I do like this and I think it's the minimum thing needed if we want to enable bootc install to be used by admins to install a domain specific system and give access to the new admins of that system.

Run this with the science officer ;) ssh key and that's it.

...lgtm

@jmarrero
Copy link
Member

jmarrero commented Feb 4, 2024

There is an argument that we should support something like a more declarative full-blown user/ssh configuration. Some higher level projects do that (e.g. kairos). There's of course also Ignition etc.

This would not be enabled by the config map/bare containers injection of files? I think if we enable that functionality anything declarative can be a very specific tool that writes configmaps/etc... like butane for bootc or so. People write a yaml and get a container with files/config map

@cgwalters
Copy link
Collaborator Author

This would not be enabled by the config map/bare containers injection of files

Right, #267 and/or configmaps would be a lot more powerful than this.

@cgwalters cgwalters merged commit 56811f4 into containers:main Feb 5, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/install Issues related to `bootc install`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants