ZIP containing single file cannot be used as an artifact due to errors in init container

[mthx]

A ZIP artifact containing a single file cannot be used as an Argo artifact. This hit me in production when a user trimmed down an input ZIP file. You get the following error from the init container:

time="2018-09-06T13:30:13Z" level=info msg="Start loading input artifacts..."
time="2018-09-06T13:30:13Z" level=info msg="Downloading artifact: aZip"
time="2018-09-06T13:30:13Z" level=info msg="curl -sS -L -o /argo/inputs/artifacts/aZip.tmp https://github.com/argoproj/argo/files/2357219/test.zip"
time="2018-09-06T13:30:14Z" level=info msg="[tar -tzf /argo/inputs/artifacts/aZip.tmp]"
time="2018-09-06T13:30:14Z" level=info msg="tar -xf /argo/inputs/artifacts/aZip.tmp -C /argo/inputs/artifacts/aZip.tmpdir"
time="2018-09-06T13:30:14Z" level=error msg="`tar -xf /argo/inputs/artifacts/aZip.tmp -C /argo/inputs/artifacts/aZip.tmpdir` failed: tar: This does not look like a tar archive\ntar: Exiting with failure status due to previous errors\n"

It's misidentified as a tar by the tar -tzf test in executor.go and so goes down the code path for extraction when it should have been handled as a simple file. This doesn't seem to be a reliable test for a tar file (the formats are very similar, so this is somewhat understandable).

The untar call only passes xf and so fails. Worryingly, if I run tar -zxf on the test input below it does nothing and claims to succeed.

This was with Argo 2.1.1 but the problematic test is also in master.

I think we should remove the z flag from the tar file test. Then it correctly notices that the ZIP isn't a tar. Compressed tars aren't supported at this point anyway, as the untar call doesn't pass the 'z' flag. A magic-number-based check might be another option. In future it might be worth considering explicit options for unpacking inputs.

Test ZIP file

Test workflow:

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: zip-artifact-
spec:
  entrypoint: zip-artifact-example
  templates:
  - name: zip-artifact-example
    inputs:
      artifacts:
      - name: aZip
        path: /tmp/a.zip
        mode: 0444
        http:
          url: https://github.com/argoproj/argo/files/2357219/test.zip
    container:
      image: debian:9.4
      command: [sh, -c]
      args: ["stat /tmp/a.zip"]

[mthx]

To add to the confusion, the default tar on OSX does support zip archives, even when you don't supply z. The suggested fix seems OK for the tar in the image though. I'll put together a PR.

[mthx]

My original summary is based on a misunderstanding but the fix is still correct.

tar without -z supports gzipped files perfectly well. Argo relies on this for processing of output artifacts obtained via docker cp. The -z is problematic as it filters via gzip which will incorrectly accept single file ZIPs and confuse tar into doing nothing.

The following terminal session shows examples run within the executor docker container:

root@1122da224bd6:/tmp# echo content > file.txt
root@1122da224bd6:/tmp# zip file.zip file.txt
  adding: file.txt (stored 0%)
root@1122da224bd6:/tmp# tar -czf file.tgz file.txt
root@1122da224bd6:/tmp# tar -tf file.tgz
file.txt
root@1122da224bd6:/tmp# tar -tf file.zip
tar: This does not look like a tar archive
tar: Exiting with failure status due to previous errors
root@1122da224bd6:/tmp# tar -tzf file.tgz
file.txt
root@1122da224bd6:/tmp# tar -tzf file.zip
root@1122da224bd6:/tmp# echo $?
0