Skip to content
This repository has been archived by the owner on Nov 24, 2022. It is now read-only.

lxc-template: make runnable by unprivileged users #447

Merged
1 commit merged into from Jan 13, 2018
Merged

lxc-template: make runnable by unprivileged users #447

1 commit merged into from Jan 13, 2018

Conversation

ghost
Copy link

@ghost ghost commented Dec 10, 2017

lxc-template needlessly require root privileges in two places:

  1. lock file location for flock
  2. failing on tar failure during rootfs extraction

For flock, it's not necessary that the lock file be in /var/lock, it
can be anywhere. Why not put it in LXC_PATH?

For the failing tar thing, that's because some device are created with
mknod which unprivileged users can't do. These device, however, are
not necessary for the container to run well. We can ignore tar's error
exit code.

I replaced the exist code check by a check for the existence of
/bin/true in rootfs. I think that it's a good indication of whether
the rootfs was extracted.

Why am I making this change? Because I'd like to add support for
unprivileged containers in vagrant-lxc but it's kind of a big change
to make at once, so I thought I'd go incrementally.

lxc-template needlessly require root privileges in two places:

1. lock file location for `flock`
2. failing on `tar` failure during rootfs extraction

For `flock`, it's not necessary that the lock file be in `/var/lock`, it
can be anywhere. Why not put it in `LXC_PATH`?

For the failing `tar` thing, that's because some device are created with
`mknod` which unprivileged users can't do. These device, however, are
not necessary for the container to run well. We can ignore `tar`'s error
exit code.

I replaced the exist code check by a check for the existence of
`/bin/true` in rootfs. I think that it's a good indication of whether
the rootfs was extracted.

Why am I making this change? Because I'd like to add support for
unprivileged containers in `vagrant-lxc` but it's kind of a big change
to make at once, so I thought I'd go incrementally.
@fgrehm
Copy link
Owner

fgrehm commented Dec 10, 2017

hey @hsoft! the project hasn't had a new release in over a year now. @ccope was helping out for a bit but I'm not sure he's still up for maintaining the project.

Having support for unprivileged users would be great and I'd be up for giving u the permissions you need to keep the project going. WDYT? :)

@ghost
Copy link
Author

ghost commented Dec 10, 2017

@fgrehm I would be happy to "keep the fire burning" as you so eloquently said in #375

I understand that you've moved to something else and I don't want to use too much of your time, but would you mind giving me high-level reviews on some design decisions I'll have to take?

Example: I'm in the process of replacing the current lxc-attach way of fetching container IP with lxc-info -iH and bump lxc requirement to v1.0+ (I doubt that anyone still use pre v1 lxc). Code-wise, I think I can manage, but I would appreciate if you could tell me something like "good idea, I don't see how it could go wrong".

If you agree, then I'd assign you to PRs, with the understanding that you'd provide high-level-only thumbs up.

@ghost
Copy link
Author

ghost commented Dec 10, 2017

As I look at the active PRs and issues in the project, I'll have to put the unprivileged containers feature on the back-burner anyway because there's a lot of high-priority issues that need attending to.

I'm guessing that after having gone through these issues, my understanding of the project will be good enough so I won't need hand holding.

Ok, I can do this. Whenever you're ready.

@fgrehm
Copy link
Owner

fgrehm commented Dec 11, 2017

I understand that you've moved to something else and I don't want to use too much of your time, but would you mind giving me high-level reviews on some design decisions I'll have to take?
If you agree, then I'd assign you to PRs, with the understanding that you'd provide high-level-only thumbs up.

Sounds good to me!

As I look at the active PRs and issues in the project, I'll have to put the unprivileged containers feature on the back-burner anyway because there's a lot of high-priority issues that need attending to.

Makes sense!

Ok, I can do this. Whenever you're ready.

Invited you as a collaborator :D

@fgrehm
Copy link
Owner

fgrehm commented Dec 11, 2017

And BTW, I'm totally fine with dropping support for LXC < 1.0 🤘

@ghost
Copy link
Author

ghost commented Dec 12, 2017

@fgrehm critical PRs were processed and we'd be ready for a v1.2.4. Do you prefer to publish vagrant-lxc to rubygems yourself? Otherwise, my username on rubygems is hsoft.

@fgrehm
Copy link
Owner

fgrehm commented Dec 12, 2017

@hsoft tks! please open up a PR with the prep for the release with changelog update + version bump and tag me in it. I'm going to try to put up automated rubygems release through travis this week and if I can't get it done will just give u permissions there as well 🍺

@ghost ghost mentioned this pull request Dec 12, 2017
@ghost ghost merged commit c8801ba into fgrehm:master Jan 13, 2018
This pull request was closed.
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.

1 participant