Added --chmod for ADD and COPY commands. Fixes #2850 and #1751

[mschneider82]

Fixes #2850 and #1751

Description

I have added --chmod feature for ADD and COPY command.

�[36mINFO�[0m[0004] ADD --chmod=644 https://xxx/test.gpg /etc/apt/trusted.gpg.d/ 
�[36mINFO�[0m[0004] RUN ls -la /etc/apt/trusted.gpg.d/test.gpg 
�[36mINFO�[0m[0007] Cmd: /bin/sh                                 
�[36mINFO�[0m[0007] Args: [-c ls -la /etc/apt/trusted.gpg.d/test.gpg] 
�[36mINFO�[0m[0007] Running: [/bin/sh -c ls -la /etc/apt/trusted.gpg.d/test.gpg] 
-rw-r--r-- 1 root root 1545 May  5  2023 /etc/apt/trusted.gpg.d/test.gpg

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

• Includes unit tests
• Adds integration tests if needed.

Reviewer Notes

• The code flow looks good.
• Unit tests and or integration tests added.

Release Notes

- kaniko now supports setting --chmod on ADD and COPY commands.

[google-cla]

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.

[mschneider82]

@aaron-prindle can you please review?

[aaron-prindle]

@mschneider82 thanks for the PR here! The PR itself LGTM, currently there is some test failures but I believe they are flakes and unrelated. Can you rebase the PR and resubmit to see if we can get the CI/CD tests green so we can merge this? Thanks

[mschneider82]

Still fails but i have tested the new Integration with LOCAL env set.

Error below from Integration Test on GitHub, with this stat command i check the permissions of the Default ADD command without --chmod

It uses the src as permissions source for chmod. It was 775 on my git Checkout, maybe different in github

            INFO[0000] Running: [/bin/sh -c test "$(stat -c "%a" /path/file666)" = "666"] 
            INFO[0000] Taking snapshot of full filesystem...        
            INFO[0000] No files were changed, appending empty layer to config. No layer added to image. 
            INFO[0000] RUN test "$(stat -c "%a" /path/file)" = "775" 
            INFO[0000] Cmd: /bin/sh                                 
            INFO[0000] Args: [-c test "$(stat -c "%a" /path/file)" = "775"] 
            INFO[0000] Running: [/bin/sh -c test "$(stat -c "%a" /path/file)" = "775"] 
            error building image: error building stage: failed to execute command: waiting for process to exit: exit status 1
FAIL
FAIL	github.com/GoogleContainerTools/kaniko/integration	488.942s
FAIL

[mschneider82]

I have removed the mentioned checks for default behaiviour for the ADD and COPY without a --chmod command. the integration test now only checks the --chmod feature.

@aaron-prindle please trigger https://github.com/GoogleContainerTools/kaniko/actions/runs/8758320746

[mschneider82]

I dont see whats wrong with the Integration test

[aaron-prindle]

I believe this is a test flake in our integration tests @mschneider82. In the past 2-3 days I have seen the below error on another PR (which was only bumping deps) as well:

        cmd.go:40: [container-diff diff --no-cache daemon://localhost:5000/docker-dockerfile_test_cache_copy_oci daemon://localhost:5000/kaniko-dockerfile_test_cache_copy_oci -q --type=file --type=metadata --json]
        cmd.go:41: time="2024-04-15T03:01:15Z" level=error msg="error retrieving image daemon://localhost:5000/docker-dockerfile_test_cache_copy_oci: retrieving image from daemon: Error response from daemon: reference does not exist"

I am still trying to root cause this atm. If I can identify the issue soon I will submit a fix and rebase (if this is related to our test infra) and if not I can merge this as is (with test flake) as I don't believe the changes in this PR are related to the test failures

flake tracking issue:
#3124

previous runs with this flake (the same PR would then pass later):
https://github.com/GoogleContainerTools/kaniko/actions/runs/8682504136/job/23807123764#step:7:73

[mschneider82]

Ok feel free to merge then, i use this patched Version since days without an issue

[aaron-prindle]

Just merged, #3131. If you rebase and push we can hopefully see some additional error information

[mschneider82]

Ok it fails because the docker command does not support chmod without buildkit:

            the --chmod option requires BuildKit. Refer to https://docs.docker.com/go/buildkit/ to learn how to build images with BuildKit enabled

in .github/workflows/integration-tests.yaml its disabled DOCKER_BUILDKIT: '0' maybe try to switch it on?

[mschneider82]

ok doing buildkit=1 for integration is a longer task to fix, but this should be a seperated task. The lagacy build will be deprecated, to it should compare to buildkit instead of legacy docker build.

[mschneider82]

I found the chmod container build, with docker it had 11 layers with kaniko just 5?

https://github.com/GoogleContainerTools/kaniko/actions/runs/8780308512/job/24089955636?pr=3119#step:7:765

To debug this it would be helpful if the build from kaniko would be also printed. currently in images.go the output is just printed when the image build fails, but not returned, in this case it would be helpful to return it and also print it when the layercheck has a difference detected.

[mschneider82]

@aaron-prindle OK i got it green, i had to enable DOCKER_BUILDKIT=1 for just this single integration test via envsMap in images.go also added the layers difference of 6. I have rebased everything. In future i think many more tests need to be fixed to use buildkit instead of classic build.

[aaron-prindle]

Thanks for your detailed explanations and investigation here @mschneider82. Will update #3124 with your findings.

LGTM, merging