fix(backend): OutPutPath dir creation mode Fixes #7629 #9946
Conversation
|
Hi @rvadim. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
/assign @chensun |
|
I have built it and tested with mutation webhook for changing container image, and it works fine after fix. |
I'm still puzzled who would the execution bits matters for file read&write. Is there anything special to the base image you're using? This error doesn't reproduce to all pipelines runs, and I'm curious what's the trigger point here. |
|
/ok-to-test |
|
I have tested with code below(based on https://pkg.go.dev/os#MkdirAll): package main
import (
"fmt"
"log"
"os"
)
func main() {
err := os.MkdirAll("/tmp/test/subdir", 0644)
if err != nil {
log.Fatal(err)
}
fmt.Println("/tmp/test/subdir created")
err = os.WriteFile("/tmp/test/subdir/testfile.txt", []byte("Hello, Gophers!"), 0644)
if err != nil {
log.Fatal(err)
}
fmt.Println("/tmp/test/subdir/testfile.txt created")
}For me, it looks like $ rm -R /tmp/test/
$ ls -al /tmp/test
ls: cannot access '/tmp/test': No such file or directory
$ go run test.go
2023/09/04 10:47:29 mkdir /tmp/test/subdir: permission denied
exit status 1
$ ls -la /tmp/test/
ls: cannot access '/tmp/test/.': Permission denied
ls: cannot access '/tmp/test/..': Permission denied
total 0
d????????? ? ? ? ? ? .
d????????? ? ? ? ? ? ..
BUT, you could emulate $ mkdir -p -m0644 /tmp/test/
$ ls -la /tmp/test/
ls: cannot access '/tmp/test/.': Permission denied
ls: cannot access '/tmp/test/..': Permission denied
total 0
d????????? ? ? ? ? ? .
d????????? ? ? ? ? ? ..
$ mkdir -p -m0644 /tmp/test/subdir
mkdir: cannot create directory ‘/tmp/test’: Permission denied
So, it does no matter MkdirAll can create subdir or not user will unable to write files to directories without execute bit. Of cause, under root it will work due to skip mode check. $ sudo ls -la /tmp/test/
total 68
drw-r----- 2 vadim vadim 4096 сен 4 10:47 .
drwxrwxrwt 37 root root 61440 сен 4 10:48 .. |
|
/retest |
|
Hi! Also I have no idea how my changes trigger problems with dependencies: Also, as far as I see all latest commits in master fail to pass test. |
The execution bit on a directory is required to enter the directory for any reason, including the creation of new files. So it is not surprising that one gets the Permission Denied error. I find it more curious why this ever worked - if it has. We have this problem with our Container Components. This is currently a blocker for us, it would be great if this could be fixed. |
I think there is something broken in |
|
Sample test doesn't block merge at this moment. Upgrading sample test infra is tracked by #9487 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chensun The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@rvadim: The following test failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/test kubeflow-pipeline-backend-test |
Description of your changes:
Try to fix #7629
Print of
ls -laafter kfp-launcher created a dirMaybe it is not complete fix, need an advise.