[PATCH 0/3] Rework git safe dir handling in GitLab CI

[Felix Moessbauer]

In 06aae60b65 ("container: Disable git safe.directory when running ..."), we
mitigated the issue of git not being able to operate on a directory that is
checked out by the GitLab CI runner. While the reason was unclear back then,
we now know that this is related to the different users of the bind-mounted
/builds dir and the container. This finally was fixed in the kas container
entry point script by completely disabling the git safe-dir handling.

Recently we got a report from a user that stumbled into the same issue as he
was using plain kas (not using the kas container). In the meantime, we added
logic in kas to detect well known CI environments and create a dedicated,
transient .gitconfig file. This series now reverts disabling of the git
safe dirs logic in the kas container (p1) and replaces it by a more fine
grained approach in kas itself (p2).

Moving the git.directory logic to kas also uncovered another issue w.r.t.
the project root directory: We check too early (before the transient
.gitconfig is injected). This is fixed by making the repo-root resolution
logic used in the IncludeHandler lazy, so the configuration is available when
resolving the root directory needed for the includes.

The whole series has been tested on GitLab CI, showing the following output
of an example job:

$ kas -l debug build kas/realtime-image-base.yml:kas/opt/distro-debian-bookworm.yml
2025-02-13 16:49:20 - INFO - kas 4.7 started
2025-02-13 16:49:20 - DEBUG - Using selector: EpollSelector
2025-02-13 16:49:20 - DEBUG - execute setup_dir
2025-02-13 16:49:20 - DEBUG - execute setup_home
2025-02-13 16:49:20 - INFO - Running on GitLab CI
2025-02-13 16:49:20 - DEBUG - Adding git safe.directory /builds/debian/meta-foo
2025-02-13 16:49:20 - DEBUG - Adding GitLab CI ssh -> https rewrites
2025-02-13 16:49:20 - DEBUG - execute init_setup_repos
2025-02-13 16:49:20 - DEBUG - /builds/debian/meta-foo/kas$ git rev-parse --show-toplevel --show-superproject-working-tree
2025-02-13 16:49:20 - DEBUG - /builds/debian/meta-foo/kas/opt$ git rev-parse --show-toplevel --show-superproject-working-tree
2025-02-13 16:49:20 - DEBUG - /builds/debian/meta-foo/kas$ git rev-parse --show-toplevel --show-superproject-working-tree
2025-02-13 16:49:20 - DEBUG - append lockfile /builds/debian/meta-foo/kas/opt/meta-foo.lock.yml
2025-02-13 16:49:20 - DEBUG - execute repo_setup_loop
2025-02-13 16:49:20 - DEBUG - Loop repo_setup_loop: execute setup_repos_step
2025-02-13 16:49:20 - DEBUG - execute finish_setup_repos
2025-02-13 16:49:20 - DEBUG - /builds/debian/meta-foo$ git rev-parse --show-toplevel --show-superproject-working-tree
2025-02-13 16:49:20 - INFO - Using /builds/debian/meta-foo as root for repository meta-foo

Best regards,
Felix Moessbauer
Siemens AG

Felix Moessbauer (3):
kas: lazy resolve top repo path
GitLab CI: add git safe dir for project root
Revert "container: Disable git safe.directory when running without
kas-container"

container-entrypoint | 11 ++---------
kas/config.py | 17 ++---------------
kas/includehandler.py | 31 ++++++++++++++++++++++++++++---
kas/libcmds.py | 18 ++++++++++++------
tests/conftest.py | 1 +
tests/test_commands.py | 1 +
6 files changed, 46 insertions(+), 33 deletions(-)

--
2.47.2

[Felix Moessbauer]

Previously the top repo path was resolved when creating the config. This
has proven to be problematic as at this point in time neither the home
nor the git / mercurial environment is set up. This makes it impossible
to configure e.g. the git safe.dir in the dynamically generated
.gitconfig file, as this is created after the Config instantiation but
before the init_setup_repos.

We now change this by adding support to lazily resolve the top repo
path. In case the IncludeHandler is created with top_repo_path=None,
this path is resolved on the first access (i.e. on the first invocation
of get_top_repo_path). By that, we can further move the config
file validation part (all configs are from the same repo) to the
IncludeHandler.

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>

---
kas/config.py | 17 ++---------------
kas/includehandler.py | 31 ++++++++++++++++++++++++++++---

2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/kas/config.py b/kas/config.py
index fbe775ecb..e386f010d 100644
--- a/kas/config.py
+++ b/kas/config.py
@@ -27,7 +27,7 @@ import os
import json
from pathlib import Path
from .repos import Repo
-from .includehandler import IncludeHandler, IncludeException
+from .includehandler import IncludeHandler
from .kasusererror import ArtifactNotFoundError
from .configschema import CONFIGSCHEMA

@@ -51,23 +51,10 @@ class Config:

self.filenames = [os.path.abspath(configfile)
for configfile in filename.split(':')]
- top_repo_path = Repo.get_root_path(
- os.path.dirname(self.filenames[0]))
-
- repo_paths = [Repo.get_root_path(os.path.dirname(configfile),
- fallback=False)
- for configfile in self.filenames]
-
- if len(set(repo_paths)) > 1:
- raise IncludeException('All concatenated config files must '
- 'belong to the same repository or all '
- 'must be outside of versioning control')

update = ctx.args.update if hasattr(ctx.args, 'update') else False

- self.handler = IncludeHandler(self.filenames,
- top_repo_path,
- not update)
+ self.handler = IncludeHandler(self.filenames, None, not update)
self.repo_dict = self._get_repo_dict()
self.repo_cfg_hashes = {}

diff --git a/kas/includehandler.py b/kas/includehandler.py
index c0ae593ce..49a6ea11f 100644
--- a/kas/includehandler.py
+++ b/kas/includehandler.py
@@ -29,6 +29,7 @@ import os
from pathlib import Path
from collections import OrderedDict
from collections.abc import Mapping
+from functools import cached_property
import functools
import logging
import json
@@ -37,6 +38,7 @@ import yaml
from jsonschema.validators import validator_for

from .kasusererror import KasUserError
+from .repos import Repo
from . import __file_version__, __compatible_file_version__, __version__
from . import CONFIGSCHEMA

@@ -130,7 +132,8 @@ class IncludeHandler:
current file, or as a dictionary. The dictionary must have a
'file' key containing the path to the include file and a 'repo'
key containing the key of the repository. The path is interpreted
- relative to the repository root path.
+ relative to the repository root path. If no top_repo_path is provided,
+ the path is lazy resolved by the first access of a method.

The includes are read and merged from the deepest level upwards.

@@ -142,16 +145,37 @@ class IncludeHandler:

def __init__(self, top_files, top_repo_path, use_lock=True):
self.top_files = top_files
- self.top_repo_path = top_repo_path
+ self._top_repo_path = top_repo_path
self.use_lock = use_lock

def get_lockfile(self, kasfile=None):
file = Path(kasfile or self.top_files[0])
return file.parent / (file.stem + '.lock' + file.suffix)

+ @cached_property
+ def top_repo_path(self):
+ """
+ Lazy resolve top repo path as we might need a prepared environment
+ """
+ return self._top_repo_path or \
+ Repo.get_root_path(os.path.dirname(self.top_files[0]))
+
def get_top_repo_path(self):
return self.top_repo_path

+ def ensure_from_same_repo(self):
+ """
+ Ensure that all concatenated config files belong to the same repository
+ """
+ repo_paths = [Repo.get_root_path(os.path.dirname(configfile),
+ fallback=False)
+ for configfile in self.top_files]
+
+ if len(set(repo_paths)) > 1:
+ raise IncludeException('All concatenated config files must '
+ 'belong to the same repository or all '
+ 'must be outside of versioning control')
+
def get_config(self, repos=None):
"""
Parameters:
@@ -302,9 +326,10 @@ class IncludeHandler:

configs = []
missing_repos = []
+ self.ensure_from_same_repo()
for configfile in self.top_files:
cfgs, reps = _internal_include_handler(configfile,
- self.top_repo_path)
+ self.get_top_repo_path())
configs.extend(cfgs)
for repo in reps:
if repo not in missing_repos:
--
2.47.2

[Felix Moessbauer]

The GitLab CI runner mounts the $CI_PROJECT_DIR git project root from
the outside but does not align the ownership of that directory with the
user of the docker container. By that, git does not allow to perform any
operation on that repository.

As this is a well-known case, we mark that directory as a safe dir if
running in GitLab CI.

Xref: https://github.com/siemens/kas/pull/141

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

kas/libcmds.py | 18 ++++++++++++------
tests/conftest.py | 1 +
tests/test_commands.py | 1 +

3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/kas/libcmds.py b/kas/libcmds.py
index e1ef8bace..62561c392 100644
--- a/kas/libcmds.py
+++ b/kas/libcmds.py
@@ -310,13 +310,19 @@ class SetupHome(Command):
if os.environ.get('GIT_CREDENTIAL_USEHTTPPATH', False):
config['credential']['useHttpPath'] = \
os.environ.get('GIT_CREDENTIAL_USEHTTPPATH')
- # in GitLab CI, add ssh -> https rewrites if no config is present
- ci_server = os.environ.get('CI_SERVER_HOST', None)
- if get_context().managed_env == ME.GITLAB_CI and ci_server and \
- not self._ssh_config_present() and \
+
+ if get_context().managed_env == ME.GITLAB_CI and \
not os.path.exists(gitconfig_host):
- logging.debug('Adding GitLab CI ssh -> https rewrites')
- self._setup_gitlab_ci_ssh_rewrite(config)
+ ci_project_dir = os.environ.get('CI_PROJECT_DIR', False)
+ if ci_project_dir:
+ logging.debug('Adding git safe.directory %s',
+ ci_project_dir)
+ config.add_value('safe', 'directory', ci_project_dir)
+
+ ci_server = os.environ.get('CI_SERVER_HOST', None)
+ if ci_server and not self._ssh_config_present():
+ logging.debug('Adding GitLab CI ssh -> https rewrites')
+ self._setup_gitlab_ci_ssh_rewrite(config)
config.write()

def execute(self, ctx):
diff --git a/tests/conftest.py b/tests/conftest.py
index 0d2aa5cef..07514a5ad 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -39,6 +39,7 @@ ENVVARS_KAS = [
'SSH_AUTH_SOCK',
'CI_SERVER_HOST',
'CI_JOB_TOKEN',
+ 'CI_PROJECT_DIR',
'GITLAB_CI',
'GITHUB_ACTIONS',
'REMOTE_CONTAINERS'
diff --git a/tests/test_commands.py b/tests/test_commands.py
index 531f9d9be..a9448f39a 100644
--- a/tests/test_commands.py
+++ b/tests/test_commands.py
@@ -98,6 +98,7 @@ def test_checkout_with_ci_rewrite(monkeykas, tmpdir):
mp.setenv('GITLAB_CI', 'true')
mp.setenv('CI_SERVER_HOST', 'github.com')
mp.setenv('CI_JOB_TOKEN', 'not-needed')
+ mp.setenv('CI_PROJECT_DIR', '/build')
kas.kas(['checkout', 'test-url-rewrite.yml'])


--
2.47.2

[Felix Moessbauer]

This reverts commit 06aae60b658eb38e2162d153ef3aa0fa59c0e2ca.

As we now handle the git safe dir part in kas, we no longer need the
global exclusion in the entrypoint.

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

container-entrypoint | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/container-entrypoint b/container-entrypoint
index b9a2f7ca7..41ce89d8d 100755
--- a/container-entrypoint
+++ b/container-entrypoint
@@ -35,15 +35,8 @@ may also need to update the host distribution (e.g. Debian Jessie -> Stretch).
EOF
fi

-if [ -z "$USER_ID" ]; then
- # Not a kas-container call
- GOSU=""
-
- # Work around gitlab-runner not aligning checked out repo ownership
- # with our builder user
- sudo git config --system safe.directory "*"
-elif [ "$USER_ID" = 0 ]; then
- # We shall run everything as root
+if [ -z "$USER_ID" ] || [ "$USER_ID" = 0 ]; then
+ # Not a kas-container call, or we shall run everything as root
GOSU=""
else
GROUP_ID=${GROUP_ID:-$(id -g)}
--
2.47.2

[Jan Kiszka]

We don't exploit the feature that top_repo_path is preset. Why providing
it then?

>
> The includes are read and merged from the deepest level upwards.
>
> @@ -142,16 +145,37 @@ class IncludeHandler:
>
> def __init__(self, top_files, top_repo_path, use_lock=True):
> self.top_files = top_files
> - self.top_repo_path = top_repo_path
> + self._top_repo_path = top_repo_path

When top_repo_path is always None, we can also drop _top_repo_path.

Looks good otherwise.

Jan

--
Siemens AG, Foundational Technologies
Linux Expert Center