Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions pkg/machine/e2e/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"os"
"path/filepath"
"regexp"
"runtime"
"strconv"
"strings"
Expand Down Expand Up @@ -124,6 +125,62 @@ var _ = Describe("podman machine init", func() {
Expect(repeatSession.errorToString()).To(ContainSubstring(fmt.Sprintf("Error: machine %q already exists", mb.names[0])))
})

It("init subid range check for rootless PinP", func() {
/* By default, a new user is assigned the following sub-ID ranges (see manual useradd):
* SUB_UID_MIN=100000, SUB_GID_MIN=100000, SUB_UID_COUNT=65536, SUB_GID_COUNT=65536
* This means the default sub-UID and sub-GID ranges are 100000–165535.
*
* When the container is run rootless by the user in WSL, ID mappings occur as follows:
* Container ID 0 (root) maps to user ID on the host.
* Container IDs 1–65536 map to IDs 100000–165535 on host (range previously mentioned).
*
* If a new user is created inside the container and used to build containers with
* (rootless PinP), it will attempt to use the default sub-ID range (100000–165535). Given
* the mapping, this means that the host must at least have a SUB_UID_COUNT and
* SUB_GID_COUNT of 165536. Since 165536 would only allow rootless PinP for the first
* user (with ID 1000), the check is run against a count of 166536 (=165536+1000) as to
* provide additional margin.
*/
Comment on lines +128 to +143
Copy link
Member

Choose a reason for hiding this comment

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

rootless PinP is not really relevant to this test at all, it could just be called check subuid/gid ranges.

But in any case adding new test cases is quite slow as they all require new Vm to be created. As such I would strongly advise to not a a new It() block for something simple as this. Instead just add a new ssh command at the end of simple init with username for example

Copy link
Author

Choose a reason for hiding this comment

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

The end goal is to fix rootles PinP, but i'll make a batch script as you suggest instead.


count_min := 166536
i := new(initMachine)
name := randomString()
session, err := mb.setName(name).setCmd(i.withImage(mb.imagePath)).run()
Expect(err).ToNot(HaveOccurred())
Expect(session).To(Exit(0))

// obtain the users subid range from the machine
ssh := new(sshMachine)
sshSession, err := mb.setName(name).setCmd(ssh.withSSHCommand([]string{"cat", "/etc/subuid", "/etc/subgid"})).run()
Expect(err).ToNot(HaveOccurred())
if sshSession.ExitCode() != 0 {
Fail(fmt.Sprintf("SSH session failed with exit code %d\nstdout:\n%s\nstderr:\n%s",
sshSession.ExitCode(),
sshSession.outputToString(),
sshSession.errorToString(),
))
}
Comment on lines +156 to +162
Copy link
Member

Choose a reason for hiding this comment

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

That doesn't add anything so please don't do that.

Copy link
Author

@dvorst dvorst Nov 20, 2025

Choose a reason for hiding this comment

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

I was hoping I could create the test without having to set everything up locally. The test is failing at this point so i was hoping to print out more information to the log, without success though. It seems I have to set up everything locally, which will require resetting the podman machine as what I could tell, which is a bit cumbersome for me as I need it for work. Hopefully I have time for this around Christmas. I was not planning on leaving this in once all tests succeed

Expect(sshSession).To(Exit(0))
output := strings.TrimSpace(sshSession.outputToString())

// subuid and subgid have the format 'USER:SUB_ID_MIN:SUB_ID_COUNT', for example
// 'user:100000:65536', only count is of interest.
re := regexp.MustCompile(`(?m):(\d+)$`)
counts := re.FindAllStringSubmatch(output, -1)

// A user must exist in order to run podman rootless, a line in both subuid and subgid
// should exist for it, so 2 lines in total.
Expect(len(counts)).To(BeNumerically(">=", 2), "expected at least 1 user/line in /etc/subuid and /etc/subgid each, got %d", len(counts))

// Verify the count. At the moment only 1 user is created in the machine. If multiple users
// are ever created, this will check that all users have a sufficient subid range.
for _, count := range counts {
n, err := strconv.Atoi(count[1])
Expect(err).ToNot(HaveOccurred())
Expect(n).To(BeNumerically(">=", count_min), "expected last number %d to be >= %d", n, count_min)
}
Comment on lines +171 to +181
Copy link
Member

Choose a reason for hiding this comment

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

There should be no reason to harden that for multiple user we only ever create one and should not create multiple. If that ever were to be the case we can adapt tests later. For now I don't see the value in complicating the check for this.

Copy link
Author

Choose a reason for hiding this comment

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

There is a count for the user and group id, so there will be a for loop anyway. The comment is meant to be informative for if a second user is ever created, and that this test will basically be run for all users.

})

It("run playbook", func() {
str := randomString()

Expand Down
2 changes: 2 additions & 0 deletions pkg/machine/wsl/declares.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ ln -fs /dev/null /etc/systemd/system/console-getty.service
ln -fs /dev/null /etc/systemd/system/systemd-oomd.socket
mkdir -p /etc/systemd/system/systemd-sysusers.service.d/
echo CREATE_MAIL_SPOOL=no >> /etc/default/useradd
sed -ir 's/SUB_UID_COUNT.*/SUB_UID_COUNT 200000/' /etc/login.defs
sed -ir 's/SUB_GID_COUNT.*/SUB_GID_COUNT 200000/' /etc/login.defs
Comment on lines +41 to +42
Copy link
Member

Choose a reason for hiding this comment

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

This is fixing this differently on WSL than compared to our providers which only set a range for the one user we create by writing subuid/subgid directly. So I would rather match that.

subUID := 100000
subUIDs := 1000000
if uid >= subUID && uid < (subUID+subUIDs) {
subUID = uid + 1
}
etcSubUID := fmt.Sprintf(`%s:%d:%d`, usrName, subUID, subUIDs)

In particular they use a range of 1000000 uids not 200000 which I think is important to keep the behaviours consistent between providers

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the intermediate review.

I'll change the count to 1 000 000 instead of 200 000. I’m quite busy with work at the moment, but I might have time to work on this around Christmas.

I'll also modify /etc/subuid and /etc/subgid directly, right after the adduser command is run, to align it with the ignition.go script.

My initial approach was based on the fact that adduser doesn’t provide an option to configure the sub-ID ranges, so these had to be set in a different way. Changing the defaults seemed to be a cleaner approach than modifying subuid/subgid directly, since that will give issues if adduser/useradd behavior ever changes (though i don't think that will ever change without backwards compatibility).

adduser -m [USER] -G wheel
mkdir -p /home/[USER]/.config/systemd/[USER]/
chown [USER]:[USER] /home/[USER]/.config
Expand Down