Create intermediate directories in COPY with correct uid and gid #2795
Conversation
|
Thanks for the PR @rhaps0dy . For the release note, would you mind deleting the block since it is not applicable here? |
| @@ -66,6 +66,11 @@ var copyTests = []struct { | |||
| sourcesAndDest: []string{"f[o][osp]", "tempCopyExecuteTest"}, | |||
| expectedDest: []string{"tempCopyExecuteTest"}, | |||
| }, | |||
| { | |||
| name: "Copy into several to-be-created directories", | |||
| sourcesAndDest: []string{"f[o][osp]", "tempCopyExecuteTest/foo/bar"}, | |||
There was a problem hiding this comment.
I understand that you included in the PR message about adding the test but it looks like this is not adding coverage for the CreateFile case with UID/GID :/
There was a problem hiding this comment.
What might be valuable here is to add an integration test at https://github.com/GoogleContainerTools/kaniko/tree/main/integration/dockerfiles, with COPY --chown= ... and it would be very helpful so that we could verify the fix when comparing in the future release.
There was a problem hiding this comment.
Yeah, you're right, it doesn't set UID/GID. I didn't know how to add that test :( I think I added this one to quickly be able to figure out how to create intermediate directories in the first place.
I'll add an integration test.
Ah you meant the release notes. Done. |
|
Done @JeromeJu . I tested it using: for IMAGE in ghcr.io/rhaps0dy/kaniko/executor:latest gcr.io/kaniko-project/executor:latest; do
docker run -it -v $(pwd):/workspace $IMAGE --context=dir:///workspace/integration/ --dockerfile=dockerfiles/Dockerfile_test_copy_chown_intermediate_dirs --no-push
donethe first should succeed and the latter should fail. |
|
Could you please authorize the workflow so we can be sure the tests pass in CI? |
Hi @rhaps0dy thanks for the update! It seems that all the CI tests have passed. Are there any other test you are referring to or intended to add? Otherwise this PR seems good to go. |
|
Awesome thank you! I'm happy with the PR, I think it's ready for merging too :) |
Fixes #1524
Description
#1524 reports that running
COPY --chown=...does not create intermediate directories with correct UID and GID. I fixed that.I'm not sure whether it fixes #2415 as well. Likely so because Add uses Copy under the hood, but I'm not sure how to add tests for that.
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Reviewer Notes
Release Notes
Describe any changes here so maintainer can include it in the release notes, or delete this block.