habsi/ansible
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ansible-test - Fix file permissions for delegation (#79932)

Browse Source 
* ansible-test - Fix file permissions for delegation

* Set more restrictive permissions for SSH key

* Check all execute bits, not just owner

* Add a breaking_changes changelog entry
This commit is contained in:
Matt Clay
2023-02-07 12:18:20 -08:00
committed by GitHub
parent c7c991e79d
commit c8c1402ff6
9 changed files with 111 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
changelogs/fragments/ansible-test-payload-file-permissions.yml Normal file
View File

@ -0,0 +1,9 @@
bugfixes:
- ansible-test - Use consistent file permissions when delegating tests to a container or remote host.
Files with any execute bit set will use permissions ``755``.
All other files will use permissions ``644``.
(Resolves issue https://github.com/ansible/ansible/issues/75079)
breaking_changes:
- ansible-test - Integration tests which depend on specific file permissions when running in an ansible-test managed
host environment may require changes. Tests that require permissions other than ``755`` or ``644``
may need to be updated to set the necessary permissions as part of the test run.

4
test/lib/ansible_test/_internal/commands/coverage/combine.py
View File

@ -34,6 +34,7 @@ from ...executor import (
from ...data import (
data_context,
PayloadConfig,
)
from ...host_configs import (
@ -82,9 +83,10 @@ def combine_coverage_files(args: CoverageCombineConfig, host_state: HostState) -
pairs = [(path, os.path.relpath(path, data_context().content.root)) for path in exported_paths]
def coverage_callback(files: list[tuple[str, str]]) -> None:
def coverage_callback(payload_config: PayloadConfig) -> None:
"""Add the coverage files to the payload file list."""
display.info('Including %d exported coverage file(s) in payload.' % len(pairs), verbosity=1)
files = payload_config.files
files.extend(pairs)
data_context().register_payload_callback(coverage_callback)

8
test/lib/ansible_test/_internal/commands/integration/__init__.py
View File

@ -90,6 +90,7 @@ from .cloud import (
from ...data import (
data_context,
PayloadConfig,
)
from ...host_configs import (
@ -214,11 +215,13 @@ def delegate_inventory(args: IntegrationConfig, inventory_path_src: str) -> None
if isinstance(args, PosixIntegrationConfig):
return
def inventory_callback(files: list[tuple[str, str]]) -> None:
def inventory_callback(payload_config: PayloadConfig) -> None:
"""
Add the inventory file to the payload file list.
This will preserve the file during delegation even if it is ignored or is outside the content and install roots.
"""
files = payload_config.files
inventory_path = get_inventory_relative_path(args)
inventory_tuple = inventory_path_src, inventory_path
@ -937,11 +940,12 @@ def command_integration_filter(args: TIntegrationConfig,
vars_file_src = os.path.join(data_context().content.root, data_context().content.integration_vars_path)
if os.path.exists(vars_file_src):
def integration_config_callback(files: list[tuple[str, str]]) -> None:
def integration_config_callback(payload_config: PayloadConfig) -> None:
"""
Add the integration config vars file to the payload file list.
This will preserve the file during delegation even if the file is ignored by source control.
"""
files = payload_config.files
files.append((vars_file_src, data_context().content.integration_vars_path))
data_context().register_payload_callback(integration_config_callback)

4
test/lib/ansible_test/_internal/commands/integration/cloud/__init__.py
View File

@ -47,6 +47,7 @@ from ....ci import (
from ....data import (
data_context,
PayloadConfig,
)
from ....docker_util import (
@ -189,13 +190,14 @@ class CloudBase(metaclass=abc.ABCMeta):
self.args = args
self.platform = self.__module__.rsplit('.', 1)[-1]
def config_callback(files: list[tuple[str, str]]) -> None:
def config_callback(payload_config: PayloadConfig) -> None:
"""Add the config file to the payload file list."""
if self.platform not in self.args.metadata.cloud_config:
return # platform was initialized, but not used -- such as being skipped due to all tests being disabled
if self._get_cloud_config(self._CONFIG_PATH, ''):
pair = (self.config_path, os.path.relpath(self.config_path, data_context().content.root))
files = payload_config.files
if pair not in files:
display.info('Including %s config: %s -> %s' % (self.platform, pair[0], pair[1]), verbosity=3)

12
test/lib/ansible_test/_internal/config.py
View File

@ -24,6 +24,7 @@ from .metadata import (
from .data import (
data_context,
PayloadConfig,
)
from .host_configs import (
@ -114,7 +115,7 @@ class EnvironmentConfig(CommonConfig):
self.dev_systemd_debug: bool = args.dev_systemd_debug
self.dev_probe_cgroups: t.Optional[str] = args.dev_probe_cgroups
def host_callback(files: list[tuple[str, str]]) -> None:
def host_callback(payload_config: PayloadConfig) -> None:
"""Add the host files to the payload file list."""
config = self
@ -123,6 +124,8 @@ class EnvironmentConfig(CommonConfig):
state_path = os.path.join(config.host_path, 'state.dat')
config_path = os.path.join(config.host_path, 'config.dat')
files = payload_config.files
files.append((os.path.abspath(settings_path), settings_path))
files.append((os.path.abspath(state_path), state_path))
files.append((os.path.abspath(config_path), config_path))
@ -225,9 +228,10 @@ class TestConfig(EnvironmentConfig):
if self.coverage_check:
self.coverage = True
def metadata_callback(files: list[tuple[str, str]]) -> None:
def metadata_callback(payload_config: PayloadConfig) -> None:
"""Add the metadata file to the payload file list."""
config = self
files = payload_config.files
if config.metadata_path:
files.append((os.path.abspath(config.metadata_path), config.metadata_path))
@ -264,8 +268,10 @@ class SanityConfig(TestConfig):
self.display_stderr = self.lint or self.list_tests
if self.keep_git:
def git_callback(files: list[tuple[str, str]]) -> None:
def git_callback(payload_config: PayloadConfig) -> None:
"""Add files from the content root .git directory to the payload file list."""
files = payload_config.files
for dirpath, _dirnames, filenames in os.walk(os.path.join(data_context().content.root, '.git')):
paths = [os.path.join(dirpath, filename) for filename in filenames]
files.extend((path, os.path.relpath(path, data_context().content.root)) for path in paths)

9
test/lib/ansible_test/_internal/core_ci.py
View File

@ -6,6 +6,7 @@ import dataclasses
import json
import os
import re
import stat
import traceback
import uuid
import time
@ -46,6 +47,7 @@ from .ci import (
from .data import (
data_context,
PayloadConfig,
)
@ -441,14 +443,19 @@ class SshKey:
key, pub = key_pair
key_dst, pub_dst = self.get_in_tree_key_pair_paths()
def ssh_key_callback(files: list[tuple[str, str]]) -> None:
def ssh_key_callback(payload_config: PayloadConfig) -> None:
"""
Add the SSH keys to the payload file list.
They are either outside the source tree or in the cache dir which is ignored by default.
"""
files = payload_config.files
permissions = payload_config.permissions
files.append((key, os.path.relpath(key_dst, data_context().content.root)))
files.append((pub, os.path.relpath(pub_dst, data_context().content.root)))
permissions[os.path.relpath(key_dst, data_context().content.root)] = stat.S_IRUSR | stat.S_IWUSR
data_context().register_payload_callback(ssh_key_callback)
self.key, self.pub = key, pub

11
test/lib/ansible_test/_internal/data.py
View File

@ -50,6 +50,13 @@ from .provider.layout.unsupported import (
)
@dataclasses.dataclass(frozen=True)
class PayloadConfig:
"""Configuration required to build a source tree payload for delegation."""
files: list[tuple[str, str]]
permissions: dict[str, int]
class DataContext:
"""Data context providing details about the current execution environment for ansible-test."""
def __init__(self) -> None:
@ -63,7 +70,7 @@ class DataContext:
self.__source_providers = source_providers
self.__ansible_source: t.Optional[tuple[tuple[str, str], ...]] = None
self.payload_callbacks: list[c.Callable[[list[tuple[str, str]]], None]] = []
self.payload_callbacks: list[c.Callable[[PayloadConfig], None]] = []
if content_path:
content = self.__create_content_layout(layout_providers, source_providers, content_path, False)
@ -173,7 +180,7 @@ class DataContext:
return self.__ansible_source
def register_payload_callback(self, callback: c.Callable[[list[tuple[str, str]]], None]) -> None:
def register_payload_callback(self, callback: c.Callable[[PayloadConfig], None]) -> None:
"""Register the given payload callback."""
self.payload_callbacks.append(callback)

1
test/lib/ansible_test/_internal/delegation.py
View File

@ -177,7 +177,6 @@ def delegate_command(args: EnvironmentConfig, host_state: HostState, exclude: li
con.run(['mkdir', '-p'] + writable_dirs, capture=True)
con.run(['chmod', '777'] + writable_dirs, capture=True)
con.run(['chmod', '755', working_directory], capture=True)
con.run(['chmod', '644', os.path.join(content_root, args.metadata_path)], capture=True)
con.run(['useradd', pytest_user, '--create-home'], capture=True)
con.run(insert_options(command, options + ['--requirements-mode', 'only']), capture=False)

69
test/lib/ansible_test/_internal/payload.py
View File

@ -27,6 +27,7 @@ from .util import (
from .data import (
data_context,
PayloadConfig,
)
from .util_common import (
@ -44,11 +45,64 @@ def create_payload(args: CommonConfig, dst_path: str) -> None:
return
files = list(data_context().ansible_source)
filters = {}
permissions: dict[str, int] = {}
filters: dict[str, t.Callable[[tarfile.TarInfo], t.Optional[tarfile.TarInfo]]] = {}
def apply_permissions(tar_info: tarfile.TarInfo, mode: int) -> t.Optional[tarfile.TarInfo]:
"""
Apply the specified permissions to the given file.
Existing file type bits are preserved.
"""
tar_info.mode &= ~(stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO)
tar_info.mode |= mode
return tar_info
def make_executable(tar_info: tarfile.TarInfo) -> t.Optional[tarfile.TarInfo]:
"""Make the given file executable."""
tar_info.mode |= stat.S_IXUSR | stat.S_IXOTH | stat.S_IXGRP
"""
Make the given file executable and readable by all, and writeable by the owner.
Existing file type bits are preserved.
This ensures consistency of test results when using unprivileged users.
"""
return apply_permissions(
tar_info,
stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH |
stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH |
stat.S_IWUSR
)
def make_non_executable(tar_info: tarfile.TarInfo) -> t.Optional[tarfile.TarInfo]:
"""
Make the given file readable by all, and writeable by the owner.
Existing file type bits are preserved.
This ensures consistency of test results when using unprivileged users.
"""
return apply_permissions(
tar_info,
stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH |
stat.S_IWUSR
)
def detect_permissions(tar_info: tarfile.TarInfo) -> t.Optional[tarfile.TarInfo]:
"""
Detect and apply the appropriate permissions for a file.
Existing file type bits are preserved.
This ensures consistency of test results when using unprivileged users.
"""
if tar_info.path.startswith('ansible/'):
mode = permissions.get(os.path.relpath(tar_info.path, 'ansible'))
else:
mode = None
if mode:
tar_info = apply_permissions(tar_info, mode)
elif tar_info.mode & (stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH):
# If any execute bit is set, treat the file as executable.
# This ensures that sanity tests which check execute bits behave correctly.
tar_info = make_executable(tar_info)
else:
tar_info = make_non_executable(tar_info)
return tar_info
if not ANSIBLE_SOURCE_ROOT:
@ -85,10 +139,15 @@ def create_payload(args: CommonConfig, dst_path: str) -> None:
# there are no extra files when testing ansible itself
extra_files = []
payload_config = PayloadConfig(
files=content_files,
permissions=permissions,
)
for callback in data_context().payload_callbacks:
# execute callbacks only on the content paths
# this is done before placing them in the appropriate subdirectory (see below)
callback(content_files)
callback(payload_config)
# place ansible source files under the 'ansible' directory on the delegated host
files = [(src, os.path.join('ansible', dst)) for src, dst in files]
@ -109,7 +168,7 @@ def create_payload(args: CommonConfig, dst_path: str) -> None:
with tarfile.open(dst_path, mode='w:gz', compresslevel=4, format=tarfile.GNU_FORMAT) as tar:
for src, dst in files:
display.info('%s -> %s' % (src, dst), verbosity=4)
tar.add(src, dst, filter=filters.get(dst))
tar.add(src, dst, filter=filters.get(dst, detect_permissions))
duration = time.time() - start
payload_size_bytes = os.path.getsize(dst_path)