Repository: Stratoscale/skipper

Issue:
Fails to tag skipper build in git subdir
```=> => naming to docker.io/library/jenkins-tools:none```

```bash
[ilia@fedora jenkins-tools]$ skipper build jenkins-tools
WARNING:root:*** Not working in a git repository ***
[skipper] Building image: jenkins-tools
INFO:skipper:Building image: jenkins-tools
[+] Building 0.1s (12/12) FINISHED                                                                                        docker:default
 => [internal] load build definition from Dockerfile.jenkins-tools                                                                  0.0s
 => => transferring dockerfile: 503B                                                                                                0.0s
 => [internal] load .dockerignore                                                                                                   0.0s
 => => transferring context: 2B                                                                                                     0.0s
 => [internal] load metadata for rackattack-nas.dc1:5000/python:3.10.12-slim-buster                                                 0.0s
 => [1/7] FROM rackattack-nas.dc1:5000/python:3.10.12-slim-buster                                                                   0.0s
 => [internal] load build context                                                                                                   0.0s
 => => transferring context: 313B                                                                                                   0.0s
 => CACHED [2/7] COPY requirements.txt /tmp/requirements.txt                                                                        0.0s
 => CACHED [3/7] RUN python -m pip install -r /tmp/requirements.txt                                                                 0.0s
 => CACHED [4/7] COPY dist/zadara_jenkins_tools-*.whl /tmp/                                                                         0.0s
 => CACHED [5/7] RUN python -m pip install /tmp/zadara_jenkins_tools-*.whl                                                          0.0s
 => CACHED [6/7] RUN echo $(eval "$(_FOO_BAR_COMPLETE=bash_source job-aligner)") >> ~/.bashrc                                       0.0s
 => CACHED [7/7] RUN rm -rf /tmp/zadara_jenkins_tools.whl /tmp/requirements.txt                                                     0.0s
 => exporting to image                                                                                                              0.0s
 => => exporting layers                                                                                                             0.0s
 => => writing image sha256:18f558fa752599d1c818f36ab6c8cef756a368d857a4ff7d91a4e1ef3bedb961                                        0.0s
 => => naming to docker.io/library/jenkins-tools:none                                                                               0.0s
[ilia@fedora jenkins-tools]$ git rev-parse HEAD
f2d5912a56a79ae31664c722212a9a4b1181c15f
[ilia@fedora jenkins-tools]$ 
```

Discussion:
IliaFeldgun: ```
      Successfully uninstalled strato-skipper-2.2.3
Successfully installed strato-skipper-2.2.4.dev1
```

```bash
[ilia@fedora jenkins-tools]$ skipper build jenkins-tools
[skipper] Building image: jenkins-tools
[+] Building 0.1s (12/12) FINISHED                                                                                        docker:default
 => [internal] load .dockerignore                                                                                                   0.0s
 => => transferring context: 2B                                                                                                     0.0s
 => [internal] load build definition from Dockerfile.jenkins-tools                                                                  0.0s
 => => transferring dockerfile: 503B                                                                                                0.0s
 => [internal] load metadata for rackattack-nas.dc1:5000/python:3.10.12-slim-buster                                                 0.0s
 => [1/7] FROM rackattack-nas.dc1:5000/python:3.10.12-slim-buster                                                                   0.0s
 => [internal] load build context                                                                                                   0.0s
 => => transferring context: 313B                                                                                                   0.0s
 => CACHED [2/7] COPY requirements.txt /tmp/requirements.txt                                                                        0.0s
 => CACHED [3/7] RUN python -m pip install -r /tmp/requirements.txt                                                                 0.0s
 => CACHED [4/7] COPY dist/zadara_jenkins_tools-*.whl /tmp/                                                                         0.0s
 => CACHED [5/7] RUN python -m pip install /tmp/zadara_jenkins_tools-*.whl                                                          0.0s
 => CACHED [6/7] RUN echo $(eval "$(_FOO_BAR_COMPLETE=bash_source job-aligner)") >> ~/.bashrc                                       0.0s
 => CACHED [7/7] RUN rm -rf /tmp/zadara_jenkins_tools.whl /tmp/requirements.txt                                                     0.0s
 => exporting to image                                                                                                              0.0s
 => => exporting layers                                                                                                             0.0s
 => => writing image sha256:18f558fa752599d1c818f36ab6c8cef756a368d857a4ff7d91a4e1ef3bedb961                                        0.0s
 => => naming to docker.io/library/jenkins-tools:f2d5912a56a79ae31664c722212a9a4b1181c15f                                           0.0s
[ilia@fedora jenkins-tools]$ 
```


Patch:
diff --git a/README.md b/README.md
index ed5f41c..c7d432e 100644
--- a/README.md
+++ b/README.md
@@ -42,11 +42,28 @@ export LANG="en_US.UTF-8"
 
 ## Note for Linux Users with Docker
 
-If you run on Linux and use Docker without sudo, Skipper will create a dedicated user inside the build container with both root and docker groups. Commands are executed on behalf of this user.
+## Running Skipper on Linux without sudo
 
-To preserve the environment (e.g., PATH), Skipper uses the `su` command with the `-m` flag. However, on Debian distros, even with the `-m` flag specified, the PATH variable may be reset. As a workaround, Skipper attempts to use `sudo -sE` (if installed) as an alternative to maintain the environment.
+If you are running Skipper on Linux without sudo, Skipper will create a dedicated user inside the build container with both root and docker groups. All commands will be executed on behalf of this user.
 
-If you prefer to use sudo, please install it in the build container. Additionally, it is required to disable `env_reset` with `secure_path` in `/etc/sudoers` Deafults.
+To preserve the environment, Skipper uses the `su` command with the `-m` flag. However, on Debian distros, the PATH variable may be reset even with the `-m` flag specified. To work around this issue, Skipper provides an alternative option using `sudo -sE`.
+
+To use `sudo -sE` as an alternative:
+
+1. Install `sudo` in the build container.
+2. Disable `env_reset` with `secure_path` in the `/etc/sudoers` defaults.
+3. Set the `SKIPPER_USE_SUDO` environment variable to `"true"`.
+
+```shell
+export SKIPPER_USE_SUDO="true"
+```
+
+```yaml
+# skipper.yaml
+
+env:
+    SKIPPER_USE_SUDO: "true"
+```
 
 **Note:** This information is crucial for a seamless experience when using Skipper with Docker on Linux.
 
diff --git a/skipper/config.py b/skipper/config.py
index 8ee1838..3983e7f 100644
--- a/skipper/config.py
+++ b/skipper/config.py
@@ -35,6 +35,8 @@ def _normalize_config(config, normalized_config):
 
 
 def _interpolate_env_vars(key):
+    if not key or isinstance(key, bool):
+        return key
     for match in findall(r'\$\(.+\)', key):
         output = check_output("echo " + match, shell=True).strip().decode("utf-8")
         if not output:
diff --git a/skipper/data/skipper-entrypoint.sh b/skipper/data/skipper-entrypoint.sh
index ef65130..7ae3d58 100755
--- a/skipper/data/skipper-entrypoint.sh
+++ b/skipper/data/skipper-entrypoint.sh
@@ -28,10 +28,8 @@ if ! [ -z "${SKIPPER_DOCKER_GID}" ];then
      usermod -G root ${SKIPPER_USERNAME}
   fi
 
-  if sudo -l -U ${SKIPPER_USERNAME} 2> /dev/null; then
-    # for debian distros (maybe for others too) -m flag resets the PATH variable
-    # so we need to use sudo -E to preserve the PATH
-    sudo -sE -u ${SKIPPER_USERNAME} $@
+  if [ "$SKIPPER_USE_SUDO" == "true" ]; then
+    sudo -sE -u ${SKIPPER_USERNAME} "$@"
   else 
     su -m ${SKIPPER_USERNAME} -c "$@"
   fi
diff --git a/skipper/git.py b/skipper/git.py
index 5098114..2766db7 100644
--- a/skipper/git.py
+++ b/skipper/git.py
@@ -1,10 +1,10 @@
-import os.path
 import logging
+import os
 import subprocess
 
 
 def get_hash(short=False):
-    if not os.path.exists('.git'):
+    if not is_git_repository():
         logging.warning('*** Not working in a git repository ***')
         return 'none'
 
@@ -22,3 +22,10 @@ def get_hash(short=False):
 def uncommitted_changes():
     """Return True is there are uncommitted changes."""
     return subprocess.call(['git', 'diff', '--quiet', 'HEAD']) != 0
+
+
+def is_git_repository():
+    if os.path.exists('.git'):
+        return True
+    command = ['git', 'rev-parse', '--is-inside-work-tree']
+    return subprocess.call(command, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) == 0


Test patch:
diff --git a/tests/test_git.py b/tests/test_git.py
index 2eed711..1f3a40a 100644
--- a/tests/test_git.py
+++ b/tests/test_git.py
@@ -1,3 +1,4 @@
+import subprocess
 import unittest
 import mock
 from skipper import git
@@ -9,33 +10,49 @@ GIT_HASH_SHORT = b'00efe97'
 
 class TestGit(unittest.TestCase):
     @mock.patch('subprocess.check_output', return_value=GIT_HASH_FULL)
-    @mock.patch('os.path.exists', return_value=True)
-    def test_get_hash_with_default_argument(self, exists_mock, check_output_mock):
+    @mock.patch('skipper.git.is_git_repository', return_value=True)
+    def test_get_hash_with_default_argument(self, is_git_repository_mock, check_output_mock):
         git_hash = git.get_hash()
-        exists_mock.assert_called_once_with('.git')
+        is_git_repository_mock.assert_called_once()
         check_output_mock.assert_called_once_with(['git', 'rev-parse', 'HEAD'])
         self.assertEqual(git_hash, GIT_HASH_FULL.decode('utf-8'))
 
     @mock.patch('subprocess.check_output', return_value=GIT_HASH_FULL)
-    @mock.patch('os.path.exists', return_value=True)
-    def test_get_full_hash(self, exists_mock, check_output_mock):
+    @mock.patch('skipper.git.is_git_repository', return_value=True)
+    def test_get_full_hash(self, is_git_repository_mock, check_output_mock):
         git_hash = git.get_hash(short=False)
-        exists_mock.assert_called_once_with('.git')
+        is_git_repository_mock.assert_called_once()
         check_output_mock.assert_called_once_with(['git', 'rev-parse', 'HEAD'])
         self.assertEqual(git_hash, GIT_HASH_FULL.decode('utf-8'))
 
     @mock.patch('subprocess.check_output', return_value=GIT_HASH_SHORT)
-    @mock.patch('os.path.exists', return_value=True)
-    def test_get_short_hash(self, exists_mock, check_output_mock):
+    @mock.patch('skipper.git.is_git_repository', return_value=True)
+    def test_get_short_hash(self, is_git_repository_mock, check_output_mock):
         git_hash = git.get_hash(short=True)
-        exists_mock.assert_called_once_with('.git')
+        is_git_repository_mock.assert_called_once()
         check_output_mock.assert_called_once_with(['git', 'rev-parse', '--short', 'HEAD'])
         self.assertEqual(git_hash, GIT_HASH_SHORT.decode('utf-8'))
 
-    @mock.patch('subprocess.check_output')
+    @mock.patch('skipper.git.is_git_repository', return_value=False)
+    def test_not_in_git_project(self, is_git_repository_mock):
+        self.assertEqual(git.get_hash(), 'none')
+        is_git_repository_mock.assert_called_once()
+
+    @mock.patch('os.path.exists', return_value=True)
+    def test_should_be_in_git_project_os_path(self, exists_mock):
+        self.assertTrue(git.is_git_repository())
+        exists_mock.assert_called_once_with('.git')
+
+    @mock.patch('subprocess.call', return_value=0)
     @mock.patch('os.path.exists', return_value=False)
-    def test_not_in_git_project(self, exists_mock, check_output_mock):
-        git_hash = git.get_hash()
+    def test_should_be_in_git_project_git(self, exists_mock, call_mock):
+        self.assertTrue(git.is_git_repository())
+        exists_mock.assert_called_once_with('.git')
+        call_mock.assert_called_once_with(['git', 'rev-parse', '--is-inside-work-tree'], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
+
+    @mock.patch('subprocess.call', return_value=1)
+    @mock.patch('os.path.exists', return_value=False)
+    def test_should_not_be_in_git_project(self, exists_mock, call_mock):
+        self.assertFalse(git.is_git_repository())
         exists_mock.assert_called_once_with('.git')
-        check_output_mock.assert_not_called()
-        self.assertEqual(git_hash, 'none')
+        call_mock.assert_called_once_with(['git', 'rev-parse', '--is-inside-work-tree'], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)