cmd/entrypoint/subcommands/cp test does not respect umask

[wlynch]

Expected Behavior

TestCp passes on main branch.

Actual Behavior

$ go test .
--- FAIL: TestCp (0.00s)
    cp_test.go:54: expected permissions 0311 for destination file but found 0310
FAIL
FAIL    github.com/tektoncd/pipeline/cmd/entrypoint/subcommands 0.017s
FAIL

Steps to Reproduce the Problem

This appears to be caused by an assumption that the system umask would be fully permissive, and the cp func would fully preserve permissions. On my system, the default umask is:

$ umask
027

This strips out the all-read permissions on the file, since os.OpenFile will set permissions post-mask -

If the file does not exist, and the O_CREATE flag is passed, it is created with mode perm (before umask).

Some potential fixes-

1. Make a manual os.Chmod after the file creation, though this would not respect any existing permissions on the file.
2. Set the umask via unix.Umask but this is also problematic since IIUC this will set it for system and would only work for unix environments.
3. Loosen the test requirements to be less sensitive to permissions.

@sbwsg Any preferences between these options?

Additional Info

• Tekton Pipeline version: efaa416

/assign @wlynch

[wlynch]

/cc @sbwsg in case the edit above didn't send a notification

[ghost]

Hm, couple alternatives spring to mind:

1. Could we infer what the user's equivalent of 0311 would be by attempting to pre-write a file with dstPermissions and then reading that back to set our expectations for the test?
2. Could we pre-calculate the eventual on-disk perms by reading the current processes' umask value and performing the calc ourselves in code? (unix-specific, example of reading the umask here: https://www.yesthatblog.com/post/0072-go-and-umask/)

(1) strikes me as the most pragmatic solution (with accompanying code comment describing why we're doing it). (2) feels like it'll be a lot of unix-specific code in the unit test. I like "loosening the test requirements" if (1) isn't feasible (or if I'm missing why it's a bad choice in the first place).

wdyt?

[wlynch]

I liked (1) at first, but after writing it, I think it's tautological so it doesn't provide much value (e.g. the permissions could be 0777 and it would still pass).

(2) is problematic for any non-unix systems, and I'm not exactly sure what happens if a unix syscall is performed on a non-unix system.

I'm thinking we can get away with checking if the resulting file has any permissions we don't expect. This loosens the check without completely removing it, but I think it is still good enough for what we need.