feat(executor): Add SYS_CHROOT to ease the setup of non-root deployments with PNS. Closes #3767

[juliusvonkohout]

…cutor.

You also need to alter the argoexec image with setuid to really use rootless containers, but that can be done independently.

Motivation #3767

@jessesuen

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
• The title of the PR is (a) conventional, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
• I've signed the CLA.
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My builds are green. Try syncing with master if they are not.
• My organization is added to USERS.md.

[CLAassistant]

All committers have signed the CLA.

[alexec]

As I understand 'SYS_CHROOT' allows the process to CHROOT into another rootfs. This is not currently needed for PNS to work for most users. You're giving this to the wait container (rather than the main container) which is good, but presumably this would allow the wait container to chroot into other containers. Only in this pod?

[juliusvonkohout]

"This is not currently needed for PNS to work for most users." i think chroot is explicitly needed for artifact collection. It is also printed in the logs and the main part in the PNS executor source code. if I block SYS_CHROOT it fails with the error cannot chroot into main container for artifact collection.

E.g. the main container writes it output to /tmp/outputs/numbers/data.
SYS_PTRACE is used to monitor the main container. As soon as the main container finishes the wait/argoexec container chroots into the main container and copies over the artifacts.

Source:
https://github.com/argoproj/argo/blob/ff4736d4a96c302bf2022312dcd493c8738b8ea7/workflow/executor/pns/pns.go#L47

	// mainFS holds a file descriptor to the main filesystem, allowing the executor to access the
	// filesystem after the main process exited
	mainFS *os.File
	// rootFS holds a file descriptor to the root filesystem, allowing the executor to exit out of a chroot
	rootFS *os.File
	// debug enables additional debugging

// enterChroot enters chroot of the main container
func (p *PNSExecutor) enterChroot() error {
	if p.mainFS == nil {
		return errors.InternalErrorf("could not chroot into main for artifact collection: container may have exited too quickly")
	}
	if err := p.mainFS.Chdir(); err != nil {
		return errors.InternalWrapErrorf(err, "failed to chdir to main filesystem: %v", err)
	}
	err := syscall.Chroot(".")
	if err != nil {
		return errors.InternalWrapErrorf(err, "failed to chroot to main filesystem: %v", err)
	}
	return nil
}

Copy artifacts

// CopyFile copies a source file in a container to a local path
func (p *PNSExecutor) CopyFile(containerID string, sourcePath string, destPath string) (err error) {
	destFile, err := os.Create(destPath)
	if err != nil {
		return err
	}
	defer func() {
		// exit chroot and close the file. preserve the original error
		deferErr := p.exitChroot()
		if err == nil && deferErr != nil {
			err = errors.InternalWrapError(deferErr)
		}
		deferErr = destFile.Close()
		if err == nil && deferErr != nil {
			err = errors.InternalWrapError(deferErr)
		}
	}()
	w := bufio.NewWriter(destFile)
	err = p.enterChroot()
	if err != nil {
		return err
	}

	err = archive.TarGzToWriter(sourcePath, w)
	if err != nil {
		return err
	}

	return nil
}

// exitChroot exits chroot
func (p *PNSExecutor) exitChroot() error {
	if err := p.rootFS.Chdir(); err != nil {
		return errors.InternalWrapError(err)
	}
	err := syscall.Chroot(".")
	if err != nil {
		return errors.InternalWrapError(err)
	}
	return nil
}

Maybe it is "not currently" needed because most users just use root containers, which have SYS_CHROOT by default? I am NOT using root containers, so i have to add the capability. I thought rootless containers are the future.

I do not see how you can chroot into another filesystem that you do not know about, but as described above chroot is already being used. Furthermore a backport to 2.7 would be amazing.

[jessesuen]

Maybe it is "not currently" needed because most users just use root containers, which have SYS_CHROOT by default? I am NOT using root containers, so i have to add the capability. I thought rootless containers are the future.

Yes I believe @juliusvonkohout is correct, and we need this capability to run the wait sidecar as non-root.

As an aside, I think for PNS, if we are able to run the PNS executor without root, I think we should make this the only or default behavior.

[alexec]

Assigning to @jessesuen.

[juliusvonkohout]

You are amazing! i hope you do a release soon :-)