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

oc-mirror does not account for umask restrictions on host machine #802

Open
BadgerOps opened this issue Feb 23, 2024 · 3 comments
Open

oc-mirror does not account for umask restrictions on host machine #802

BadgerOps opened this issue Feb 23, 2024 · 3 comments

Comments

@BadgerOps
Copy link
Contributor

BadgerOps commented Feb 23, 2024

Version

$ oc-mirror version
4.12 - 4.15-rc.4

What happened?

Hi team!

We are using oc-mirror in a fairly restricted environment, with a default umask of 0077 on both our internet connected, and disconnected environment hosts. This has resulted in some confusing, unexpected issues - specifically with the redhat-operator-index container.

I don't have full context on the exact process of how oc-mirror handles the export, tar/un-tar and finally import of these imagest into an offline quay instance, but I do see the impacts of system default umask settings.

Looking at some areas of the oc-mirror codebase, I do see where explicit chmod is ran on several directories at creation time, so my assumption is that there is an expectation of correct permissions being set

However: os.Mkdir("./example/directory", 0775) is not respected by the system default umask, and in our case with a 0077 umask, that directory would have 0700 permissions.

I believe it would make sense to take advantage of golang.org/x/sys/unix to handle setting the umask in the session oc-mirror is ran, then setting it back.

Here's an example borrowed from this blog post:

package main

import (
	"fmt"
	"strconv"
	"strings"

	"golang.org/x/sys/unix"
)

func main() {
	fmt.Println("Checking umask of current session:")

	checkUmask()

	var forcedUmask int = 0022

	fmt.Println("Setting umask to more permissive 0022")
	savedUmask := unix.Umask(forcedUmask)
	checkUmask()
	_ = unix.Umask(savedUmask) // Return the umask to the original
}

func checkUmask() {
	currentUmask := unix.Umask(0)

	umaskOctal := strconv.FormatInt(int64(currentUmask), 8) // Convert the uint32 to int64 and format as octal
	desiredLength := len("0022")                            // Set desired length of output (we want 0022, not just '22')
	leadingZeros := strings.Repeat("0", desiredLength-len(umaskOctal))
	fmt.Printf("The current session umask is: %s\n", leadingZeros+umaskOctal)
}

Then, we run the program:

umask
077
❯ go run .
Checking umask of current session:
The current session umask is: 0077
Setting umask to more permissive 0022
The current session umask is: 0022

I believe an addition of session umask handling, with appropriate logging would greatly reduce some of the issues that impact Red Hat customers, and support personnel who are impacted by this problem.

I also believe it is fair to assume that a large majority of oc-mirror users are in some form of restricted environment, especially with the inroads Open Shift is making in the gvt/dod space, so these impacts are going to occur more frequently.

What did you expect to happen?

container and operator images to be migrated from the internet to a disconnected quay instance reliably, and correctly. (as in, correct permissions retained)

How to reproduce it (as minimally and precisely as possible)?

use oc-mirror to move redhat/redhat-operator-index from an internet connected system to an offline system, that are both set to a system umask of 0077

deploy that new redhat-operator-index container in an OCP cluster

observe the container go into crashLoopBackoff as it cannot read/write to /configs since that dir is created with 0700 vs 0755 permissions.

Anything else we need to know?

There are quite a few OCPBUG tickets, and Red Hat support cases covering this issue

References

https://issues.redhat.com/browse/OCPBUGS-27125 ( the exact issue we're impacted by)
https://issues.redhat.com/browse/OCPBUGS-26078 (parent ticket)
https://access.redhat.com/solutions/7006771

I am happy to work with the team on testing some strategies for implementing a umask-aware fix to help alleviate the issues we're running into when using oc-mirror to move images and operators into our disconnected environment.

Many thanks,

-BadgerOps

@BadgerOps
Copy link
Contributor Author

Any thoughts on this as a potential path forward? As we see an increase in disconnected OCP deployments, in hardened environments, I believe it makes sense to ensure the reliability and ease-of-use of the surrounding tooling is addressed.

I've spent a little time reading through the codebase, but unfortunately do not have a good enough grasp on the flow to submit an example PR at this point. I'm hoping someone on the team with better context can help out here!

Thanks!

@dmc5179
Copy link

dmc5179 commented Mar 27, 2024

golang mkdir documentation shows that it respects the system umask. We would likely need to use something like os.Chmod after running os.Mkdir.

https://pkg.go.dev/os#Mkdir

@BadgerOps
Copy link
Contributor Author

BadgerOps commented Mar 28, 2024

Do you think os.Chmod is a better route than temporarily setting 0022 umask in the session for oc-mirror ?

Here's where my ignorance of some of the internals is showing - I've been manually setting umask before running oc-mirror, and it works well enough, but is frustrating when I forget to set it.

Edit: Dan, I just saw your comments on the OCPBUGS Jira case, makes sense to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants