Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

file executable permissions check error on macos in docker container #5945

Closed
eeslook opened this issue Jun 1, 2024 · 4 comments · Fixed by #5946
Closed

file executable permissions check error on macos in docker container #5945

eeslook opened this issue Jun 1, 2024 · 4 comments · Fixed by #5946
Labels

Comments

@eeslook
Copy link
Contributor

eeslook commented Jun 1, 2024

Describe the bug
On macOS Sonoma v14.5, it was discovered that when using avocado in a Docker container to run scripts without execute permissions, they are considered to have executable permissions.

Steps to reproduce

  1. make a Dockfile name Dockerfile-test with flow content:
FROM python:3.9
RUN python -m pip install --upgrade pip && \
    pip install avocado_framework
  1. run docker build commd: docker build -t avocado-test:v1 -f Dockerfile-test .
  2. make a test file named example/00-most-simple-workflow-only-must-options.yml at current dir
  3. run command chmod -x example/00-most-simple-workflow-only-must-options.yml
  4. run command docker run -ti -v $PWD:/it avocado-test:v1 bash
  5. run command cd /it && avocado -V list example/00-most-simple-workflow-only-must-options.yml , return:

Type      Test                                                  Tag(s)
exec-test example/00-most-simple-workflow-only-must-options.yml

Resolver             Reference                                             Info
avocado-instrumented example/00-most-simple-workflow-only-must-options.yml File "example/00-most-simple-workflow-only-must-options.yml" does not end with ".py"
python-unittest      example/00-most-simple-workflow-only-must-options.yml File "example/00-most-simple-workflow-only-must-options.yml" does not end with ".py"
runnable-recipe      example/00-most-simple-workflow-only-must-options.yml File "example/00-most-simple-workflow-only-must-options.yml" does not end with ".json"
runnables-recipe     example/00-most-simple-workflow-only-must-options.yml File "example/00-most-simple-workflow-only-must-options.yml" does not end with ".json"

TEST TYPES SUMMARY
==================
exec-test: 1
root@59e444cb0603:/it# 

Expected behavior
I have an avocado plugin for example/00-most-simple-workflow-only-must-options.yml to run the scripts.
I have exec-test script too, I hope the yml file run with my avocado plugin and the excutable files run with exec-test.

Current behavior
I run the scripts on Linux with docker is ok, but failed when on macos with docker, because the exec-test runner matched first.

I have both exec-test and workflow tests in the testsuites.

System information (please complete the following information):

  • OS: uname -a , Darwin mymacbook 23.5.0 Darwin Kernel Version 23.5.0: Wed May 1 20:13:18 PDT 2024; root:xnu-10063.121.3~5/RELEASE_ARM64_T6030 arm64
  • Avocado version: Avocado 105.0
  • Avocado installation method: pip

Additional information
I debug the script in macos in the docker with command python -m pdb /usr/local/bin/avocado -V list ist example/00-most-simple-workflow-only-must-options.yml and diff with same step at linunx, get the point:

(Pdb) n
> /usr/local/lib/python3.9/site-packages/avocado/plugins/resolvers.py(44)resolve()
-> criteria_check = check_file(
(Pdb) a
self = <avocado.plugins.resolvers.ExecTestResolver object at 0xffffafa62f10>
reference = 'example/00-most-simple-workflow-only-must-options.yml'
(Pdb) p os.system("ls " + reference)
example/00-most-simple-workflow-only-must-options.yml
0
(Pdb) p os.system("ls -l " + reference)
-rw-rw-rw- 1 root root 156 Oct 23  2023 example/00-most-simple-workflow-only-must-options.yml
0
(Pdb) ll
 42  	    def resolve(self, reference):
 43  	
 44  ->	        criteria_check = check_file(
 45  	            reference,
 46  	            reference,
 47  	            suffix=None,
 48  	            type_name="executable file",
 49  	            access_check=os.R_OK | os.X_OK,
 50  	            access_name="executable",
 51  	        )
 52  	        if criteria_check is not True:
 53  	            return criteria_check
 54  	
 55  	        runnable = Runnable("exec-test", reference, assets=get_file_assets(reference))
 56  	        return ReferenceResolution(
 57  	            reference, ReferenceResolutionResult.SUCCESS, [runnable]
 58  	        )
(Pdb) n
> /usr/local/lib/python3.9/site-packages/avocado/plugins/resolvers.py(45)resolve()
-> reference,
(Pdb) 
> /usr/local/lib/python3.9/site-packages/avocado/plugins/resolvers.py(46)resolve()
-> reference,
(Pdb) 
> /usr/local/lib/python3.9/site-packages/avocado/plugins/resolvers.py(47)resolve()
-> suffix=None,
(Pdb) 
> /usr/local/lib/python3.9/site-packages/avocado/plugins/resolvers.py(48)resolve()
-> type_name="executable file",
(Pdb) 
> /usr/local/lib/python3.9/site-packages/avocado/plugins/resolvers.py(49)resolve()
-> access_check=os.R_OK | os.X_OK,
(Pdb) 
> /usr/local/lib/python3.9/site-packages/avocado/plugins/resolvers.py(50)resolve()
-> access_name="executable",
(Pdb) 
> /usr/local/lib/python3.9/site-packages/avocado/plugins/resolvers.py(44)resolve()
-> criteria_check = check_file(
(Pdb) 
> /usr/local/lib/python3.9/site-packages/avocado/plugins/resolvers.py(52)resolve()
-> if criteria_check is not True:
(Pdb) p criteria_check
True
(Pdb) p os.system("ls -l " + reference)
-rw-rw-rw- 1 root root 156 Oct 23  2023 example/00-most-simple-workflow-only-must-options.yml
0
(Pdb) exit

By directly reading the file's permission bits, the stat method can provide more accurate permission check results, especially in cases where user context and file system characteristics might affect the behavior of os.access. This method is closer to the underlying implementation of the file system, thus providing consistent results across different environments (such as inside and outside Docker containers). After entering the container using docker exec -it container bash, use the stat command to check the file permission bits:

% docker exec -ti 7d121d1f0a8d bash
root@7d121d1f0a8d:/# cd /it
root@7d121d1f0a8d:/it# python
Python 3.9.16 (main, Feb  4 2023, 15:35:00) 
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> reference = 'example/00-most-simple-workflow-only-must-options.yml'
>>> os.access(reference, os.X_OK)
True
>>> import stat
>>> os.system("ls -l " + reference)
-rw-rw-rw- 1 root root 156 Oct 23  2023 example/00-most-simple-workflow-only-must-options.yml
0
>>> st = os.stat(reference)
>>> bool(st.st_mode & (stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH))
False
>>> 
>>> exit()
root@7d121d1f0a8d:/it# 
root@7d121d1f0a8d:/it# pip list | grep avocado
avocado-framework 105.0
@eeslook eeslook added the bug label Jun 1, 2024
@mr-avocado mr-avocado bot moved this to Triage in Default project Jun 1, 2024
eeslook added a commit to eeslook/avocado that referenced this issue Jun 1, 2024
…iner

 On macOS Sonoma v14.5, it was discovered that when using avocado in a Docker container to run scripts without execute permissions, they are considered to have executable permissions.

By directly reading the file's permission bits, the `stat` method can provide more accurate permission check results, especially in cases where user context and file system characteristics might affect the behavior of `os.access`. This method is closer to the underlying implementation of the file system, thus providing consistent results across different environments (such as inside and outside Docker containers). After entering the container using `docker exec -it container bash`, use the `stat` command to check the file permission bits.

 Reference: avocado-framework#5945
Signed-off-by:  Kui Li <[email protected]>
eeslook added a commit to eeslook/avocado that referenced this issue Jun 1, 2024
The current implementation of the check_file function has an incorrect logic for checking file permissions. This merge request updates the function to correctly verify the required permissions.

Reference: avocado-framework#5945
Signed-off-by:  Kui Li <[email protected]>
@richtja
Copy link
Contributor

richtja commented Jun 4, 2024

Hi @eeslook, IIUIC you created your own resolver which can resolve .yml files into tests. Then you have suite with your YAML files and executable files, but the avocado will resolve the YAML files as exec-test because it uses exec-test resolver before your own YAML resolver, am I right? If this is the case, then you can set priority of your plugin to VERY_HIGH. It will ensure that your plugin will always resolve the files before the exec-test resolver. For more info, see https://avocado-framework.readthedocs.io/en/latest/guides/contributor/chapters/plugins.html#plugins-execution-order

@eeslook
Copy link
Contributor Author

eeslook commented Jun 6, 2024

Hi @eeslook, IIUIC you created your own resolver which can resolve .yml files into tests. Then you have suite with your YAML files and executable files, but the avocado will resolve the YAML files as exec-test because it uses exec-test resolver before your own YAML resolver, am I right? If this is the case, then you can set priority of your plugin to VERY_HIGH. It will ensure that your plugin will always resolve the files before the exec-test resolver. For more info, see https://avocado-framework.readthedocs.io/en/latest/guides/contributor/chapters/plugins.html#plugins-execution-order

Thank you very much for your reply! I indeed created a custom resolver to handle .yml files and convert them into tests. Before submitting this bug, I was aware of the plugin priority settings and realized that adjusting the priority could serve as a workaround for this issue. However, considering that there are currently issues with exec-test permission checks within Docker images on macOS systems, and given that our project also includes other types of files such as .py and bins, with some .py files requiring specific plugins for parsing and others needing the exec-test, if the determination of file executable permissions is not accurate, it will always pose a risk. Therefore, I decided to report this issue and have also proposed a code-level fix.

@richtja
Copy link
Contributor

richtja commented Jun 7, 2024

Hi @eeslook, IIUIC you created your own resolver which can resolve .yml files into tests. Then you have suite with your YAML files and executable files, but the avocado will resolve the YAML files as exec-test because it uses exec-test resolver before your own YAML resolver, am I right? If this is the case, then you can set priority of your plugin to VERY_HIGH. It will ensure that your plugin will always resolve the files before the exec-test resolver. For more info, see https://avocado-framework.readthedocs.io/en/latest/guides/contributor/chapters/plugins.html#plugins-execution-order

Thank you very much for your reply! I indeed created a custom resolver to handle .yml files and convert them into tests. Before submitting this bug, I was aware of the plugin priority settings and realized that adjusting the priority could serve as a workaround for this issue. However, considering that there are currently issues with exec-test permission checks within Docker images on macOS systems, and given that our project also includes other types of files such as .py and bins, with some .py files requiring specific plugins for parsing and others needing the exec-test, if the determination of file executable permissions is not accurate, it will always pose a risk. Therefore, I decided to report this issue and have also proposed a code-level fix.

I see, I just have looked at your #5946, and we can discuss the details there. Thank you very much for your contribution.

@richtja richtja moved this from Triage to In progress in Default project Jun 10, 2024
@eeslook
Copy link
Contributor Author

eeslook commented Jun 11, 2024

ok

eeslook added a commit to eeslook/avocado that referenced this issue Jun 13, 2024
 On macOS Sonoma v14.5, it was discovered that when using avocado in a Docker container to run scripts without execute permissions, they are considered to have executable permissions.

By directly reading the file's permission bits, the `stat` method can provide more accurate permission check results, especially in cases where user context and file system characteristics might affect the behavior of `os.access`. This method is closer to the underlying implementation of the file system, thus providing consistent results across different environments (such as inside and outside Docker containers). After entering the container using `docker exec -it container bash`, use the `stat` command to check the file permission bits.

Reference: avocado-framework#5945
Signed-off-by:  Kui Li <[email protected]>
eeslook added a commit to eeslook/avocado that referenced this issue Jun 14, 2024
On macOS Sonoma v14.5, it was discovered that when using avocado in a Docker container to run scripts without execute permissions, they are considered to have executable permissions.

By directly reading the file's permission bits, the 4017715918 919 crw--w---- 1 likui34 tty 268435457 0 "Jun 14 17:41:15 2024" "Jun 14 17:41:16 2024" "Jun 14 17:41:16 2024" "Jan  1 08:00:00 1970" 65536 0 0 (stdin) method can provide more accurate permission check results, especially in cases where user context and file system characteristics might affect the behavior of . This method is closer to the underlying implementation of the file system, thus providing consistent results across different environments (such as inside and outside Docker containers). After entering the container using , use the 4017715918 919 crw--w---- 1 likui34 tty 268435457 0 "Jun 14 17:41:15 2024" "Jun 14 17:41:16 2024" "Jun 14 17:41:16 2024" "Jan  1 08:00:00 1970" 65536 0 0 (stdin) command to check the file permission bits.

Reference: avocado-framework#5945
Signed-off-by: Kui Li <[email protected]>
eeslook added a commit to eeslook/avocado that referenced this issue Jun 14, 2024
On macOS Sonoma v14.5, it was discovered that when using avocado in a Docker container
to run scripts without execute permissions, they are considered to have executable permissions.

By directly reading the file's permission bits, the stat method can provide more accurate
permission check results, especially in cases where user context and file system
characteristics might affect the behavior of os.access. This method is closer to the
underlying implementation of the file system, thus providing consistent results across
different environments (such as inside and outside Docker containers). After entering the
container using docker exec -it container bash, use the stat command to check the
file permission bits.

Reference: avocado-framework#5945
Signed-off-by: Kui Li <[email protected]>
eeslook added a commit to eeslook/avocado that referenced this issue Jun 19, 2024
On macOS Sonoma v14.5, it was discovered that when using avocado in a Docker container
to run scripts without execute permissions, they are considered to have executable permissions.

By directly reading the file's permission bits, the stat method can provide more accurate
permission check results, especially in cases where user context and file system
characteristics might affect the behavior of os.access. This method is closer to the
underlying implementation of the file system, thus providing consistent results across
different environments (such as inside and outside Docker containers). After entering the
container using docker exec -it container bash, use the stat command to check the
file permission bits.

Reference: avocado-framework#5945
Signed-off-by: likui <[email protected]>
eeslook added a commit to eeslook/avocado that referenced this issue Jun 19, 2024
On macOS Sonoma v14.5, it was discovered that when using avocado in a Docker container
to run scripts without execute permissions, they are considered to have executable permissions.

By directly reading the file's permission bits, the stat method can provide more accurate
permission check results, especially in cases where user context and file system
characteristics might affect the behavior of os.access. This method is closer to the
underlying implementation of the file system, thus providing consistent results across
different environments (such as inside and outside Docker containers). After entering the
container using docker exec -it container bash, use the stat command to check the
file permission bits.

Reference: avocado-framework#5945
Signed-off-by: Kui Li <[email protected]>
@github-project-automation github-project-automation bot moved this from In progress to Done 106 in Default project Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants