Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[CERTTF-413] fix: include chmod method in the tarfile patch #368

Merged
merged 2 commits into from
Oct 3, 2024

Conversation

boukeas
Copy link
Contributor

@boukeas boukeas commented Oct 3, 2024

Description

This PR fixes a minor issue in how tarfile is patched for Python 3.8 that allows directories to be included as attachments.

Specifically chmod was previously left unpatched, using the native version which was unforgiving when the mode was None. However, the filtered mode for directories is actually None, so the unpatched version of chmod raised an error when unpacking directories. This PR patches chmod (in line with Python's 3.8.17 tarfile) so that directories can also be extracted and therefore used as attachments.

Resolved issues

Resolves CERTTF-413.

Tests

Submitted the job below in a local Testflinger deployment:

job_queue: myqueue
test_data:
  attachments:
    - local: hwcert-jenkins-tools/install_tools.sh
      agent: install_tools.sh
    - local: hwcert-jenkins-tools/kernel-switcher/
      agent: switcher/
  test_cmds: |
    ls -alR .

With the following results (for the test phase):

.:
total 28
drwxrwxr-x 3 boukeas boukeas 4096 Oct  3 12:33 .
drwxrwxr-x 4 boukeas boukeas 4096 Oct  3 12:33 ..
drwxrwxr-x 3 boukeas boukeas 4096 Oct  3 12:33 attachments
-rw-rw-r-- 1 boukeas boukeas    0 Oct  3 12:33 device-connector-error.json
-rw-rw-r-- 1 boukeas boukeas  266 Oct  3 12:33 testflinger.json
-rw-rw-r-- 1 boukeas boukeas    2 Oct  3 12:33 testflinger-outcome.json
-rw-rw-r-- 1 boukeas boukeas  147 Oct  3 12:33 test.log
-rwxrwxr-x 1 boukeas boukeas   22 Oct  3 12:33 tf_cmd_script

./attachments:
total 12
drwxrwxr-x 3 boukeas boukeas 4096 Oct  3 12:33 .
drwxrwxr-x 3 boukeas boukeas 4096 Oct  3 12:33 ..
drwxrwxr-x 3 boukeas boukeas 4096 Oct  3 12:33 test

./attachments/test:
total 16
drwxrwxr-x 3 boukeas boukeas 4096 Oct  3 12:33 .
drwxrwxr-x 3 boukeas boukeas 4096 Oct  3 12:33 ..
-rwxr-xr-x 1 boukeas boukeas 2802 Oct  3 11:43 install_tools.sh
drwx------ 2 boukeas boukeas 4096 Jun 21 13:04 switcher

./attachments/test/switcher:
total 56
drwx------ 2 boukeas boukeas  4096 Jun 21 13:04 .
drwxrwxr-x 3 boukeas boukeas  4096 Oct  3 12:33 ..
-rw-r--r-- 1 boukeas boukeas  1109 Jun 21 13:04 README.md
-rwxr-xr-x 1 boukeas boukeas  7068 Jun 21 13:04 switch_kernel.py
-rwxr-xr-x 1 boukeas boukeas  6476 Jun 21 13:04 test_in_lxd_vm.py
-rw-r--r-- 1 boukeas boukeas 25453 Jun 21 13:04 test_switch_kernel.py

This allows for directories to be extracted. The filtered `mode`
for directories is `None` and the unpatched `chmod` is not
forgiving of that.
@boukeas boukeas requested review from plars and a team October 3, 2024 11:41
@boukeas boukeas marked this pull request as ready for review October 3, 2024 11:42
Copy link
Collaborator

@plars plars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah, that's all we needed? nice work sorting that out! I'm really hopeful that we can upgrade SOON (ish) to get on the new infrastructure, using the new charm, and we can stop carrying things like this. In the meantime, thanks for all your diligence in keeping this working!

@boukeas boukeas merged commit a634fc1 into main Oct 3, 2024
2 checks passed
@boukeas boukeas deleted the CERTTF-413-tarfile-fix branch October 3, 2024 15:01
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants