community.general icon indicating copy to clipboard operation
community.general copied to clipboard

hit issue with module community.general.iso_extract

Open ZouYuhua opened this issue 1 year ago • 9 comments

Summary

hit issue with module community.general.iso_extract. You could get the ISO from http://releases.ubuntu.com/jammy/

Issue Type

Bug Report

Component Name

community.general.iso_extract

Ansible Version

#ansible --version ansible 2.10.8 config file = None configured module search path = ['/Users/zouy/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules'] ansible python module location = /usr/local/Cellar/ansible/3.3.0/libexec/lib/python3.9/site-packages/ansible executable location = /usr/local/bin/ansible python version = 3.9.4 (default, Apr 5 2021, 01:50:46) [Clang 12.0.0 (clang-1200.0.32.29)]

Community.general Version

ansible-galaxy collection list community.general

/usr/local/Cellar/ansible/3.3.0/libexec/lib/python3.9/site-packages/ansible_collections

Collection Version


community.general 2.5.1

Configuration

$ ansible-config dump --only-changed

No output for this command. That means we have nothing changed

OS / Environment

Mac 11.6.8 with 7zip installed

Steps to Reproduce

- name: iso_get_files
  hosts: localhost
  gather_facts: no
  vars:
     test_files_dir: "/Users/zouy/Documents"
  tasks:
    - name: "Get files from ISO"
       community.general.iso_extract:
         image: "{{ test_files_dir }}/ubuntu-22.04-desktop-amd64.iso"
         dest: "/tmp/iso_extract"
         files:
           - "/preseed/ubuntu.seed"
           - "/casper/initrd"
        register: iso_get_files

    - name: "Display the ISO customized result"
      debug: var=iso_get_files

    - name: Test iso_get_files
      assert:
        that:
        - iso_get_files is not failed

Expected Results

Get the specific files from ISO and store to local directory.

Actual Results

fatal: [localhost]: FAILED! => {
    "changed": false,
    "dest": "/tmp/iso_extract",
    "files": [],
    "gid": 0,
    "group": "wheel",
    "image": "/Users/zouy/Documents/ubuntu-22.04-desktop-amd64.iso",
    "invocation": {
        "module_args": {
            "dest": "/tmp/iso_extract",
            "executable": null,
            "files": [
                "/preseed/ubuntu.seed",
                "/casper/initrd"
            ],
            "force": true,
            "image": "/Users/zouy/Documents/ubuntu-22.04-desktop-amd64.iso"
        }
    },
    "mode": "0755",
    "msg": "Failed to extract '/preseed/ubuntu.seed' from ISO image",
    "owner": "root",
    "size": 64,
    "state": "directory",
    "uid": 0
}

PLAY RECAP *********************************************************************************************************
localhost                  : ok=0    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0   

Code of Conduct

  • [X] I agree to follow the Ansible Code of Conduct

ZouYuhua avatar Sep 13 '22 07:09 ZouYuhua

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot avatar Sep 13 '22 07:09 ansibullbot

cc @dagwieers @jhoekx @ribbons click here for bot help

ansibullbot avatar Sep 13 '22 07:09 ansibullbot

It can be fixed when change

  "files": [
                "/preseed/ubuntu.seed",
                "/casper/initrd"
   ],

to
   "files": [
                "preseed/ubuntu.seed",
                "casper/initrd"
   ],

ZouYuhua avatar Sep 13 '22 08:09 ZouYuhua

I'm not sure if this is a bug really, not having a leading slash for the extracted files matches the example in the documentation.

ribbons avatar Sep 13 '22 15:09 ribbons

@ribbons I agree this does not look like a bug, as it matches the example. However, it might be interesting to mention explicitly in the documentation that the files should be listed WITHOUT a leading slash.

russoz avatar Sep 17 '22 00:09 russoz

@ZouYuhua just for the record, your stated ansible version was 2.10.8, but then the path for the collection says .../ansible/3.3.0/.... Won't make a difference for this issue, just pointing out so that you double check on that in future occasions.

russoz avatar Sep 17 '22 00:09 russoz

actually, taking a quick glance at the code, I think the problem happens around here (the try stmt is line 172):

    try:
        for f in extract_files:
            tmp_src = os.path.join(tmp_dir, f)
            if not os.path.exists(tmp_src):
                module.fail_json(msg="Failed to extract '%s' from ISO image" % f, **result)

I did a quick check and:

>>> os.path.join('/tmp/aaa', 'bbb')
'/tmp/aaa/bbb'
>>> os.path.join('/tmp/aaa', '/bbb')
'/bbb'

We might be able to accommodate leading slashes by

        for f in extract_files:
            f = f.lstrip('/')
            tmp_src = os.path.join(tmp_dir, f)

russoz avatar Sep 17 '22 01:09 russoz

Great, also looks like os.path.exists() (and hopefully by extension the rest of the python file/path handling code :smile:) is tolerant of double slashes:

>>> os.path.exists('/home/matt/test')
True
>>> os.path.exists('/home/matt//test')
True

So we could go for the even simpler option of:

        for f in extract_files:
            tmp_src = '%s/%s' % (tmp_dir, f)

Shall I put together a PR?

ribbons avatar Sep 17 '22 18:09 ribbons

I think removing os.sep from the left of f would be the best solution, since that would even work if this code is used on platforms with other path separators. Using string concatenation is a hack that works on every Unix-like environment I know, but I think it feels more like a hack than f.lstrip(os.sep).

felixfontein avatar Sep 18 '22 10:09 felixfontein

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot avatar Nov 05 '22 10:11 ansibullbot