fix(issue-30): use uid/gid from --user with --deploy copy

during pipelines `--deploy copy` respect a `--user <uid>[:gid]` by adding tar flags `--numeric-owner --owner=:<uid> [--group=:<gid>]` to the tar creations.
ktomk · Oct 11, 2024 · afc6092 · afc6092
1 parent af943ad
commit afc6092
Show file tree

Hide file tree

Showing 9 changed files with 224 additions and 14 deletions.
diff --git a/src/Runner/Containers/StepContainer.php b/src/Runner/Containers/StepContainer.php
@@ -198,7 +198,7 @@ public function obtainUserOptions()
             return $userOpts;
         }
 
-        $userOpts = array('--user', $user);
+        $userOpts = array('--user', $user->toString());
 
         if (LibFs::isReadableFile('/etc/passwd') && LibFs::isReadableFile('/etc/group')) {
             $userOpts[] = '-v';

diff --git a/src/Runner/Docker/Provision/TarCopier.php b/src/Runner/Docker/Provision/TarCopier.php
@@ -8,6 +8,7 @@
 use Ktomk\Pipelines\Lib;
 use Ktomk\Pipelines\LibFs;
 use Ktomk\Pipelines\LibTmp;
+use Ktomk\Pipelines\Runner\Opts\User;
 use Ktomk\Pipelines\Value\SideEffect\DestructibleString;
 
 /**
@@ -36,10 +37,11 @@ class TarCopier
      * @param string $id container id
      * @param string $source directory to obtain file-properties from (user, group)
      * @param string $target directory to create within container with those properties
+     * @param array $tarFlags flags for tar, optional
      *
      * @return int status
      */
-    public static function extMakeEmptyDirectory(Exec $exec, $id, $source, $target)
+    public static function extMakeEmptyDirectory(Exec $exec, $id, $source, $target, array $tarFlags = array())
     {
         if ('/' === $target) {
             return 0;
@@ -57,7 +59,7 @@ public static function extMakeEmptyDirectory(Exec $exec, $id, $source, $target)
         LibFs::symlinkWithParents($source, $tmpDir . $target);
 
         $cd = Lib::cmd('cd', array($tmpDir . '/.'));
-        $tar = Lib::cmd('tar', array('c', '-h', '-f', '-', '--no-recursion', '.' . $target));
+        $tar = Lib::cmd('tar', array('c', '-h', '-f', '-', '--no-recursion', $tarFlags, '.' . $target));
         $dockerCp = Lib::cmd('docker ', array('cp', '-', $id . ':/.'));
 
         return $exec->pass("{$cd} && {$tar} | {$dockerCp}", array());
@@ -78,17 +80,18 @@ public static function extMakeEmptyDirectory(Exec $exec, $id, $source, $target)
      * @param string $id container id
      * @param string $source directory
      * @param string $target directory to create within container
+     * @param array $tarFlags flags for tar, optional
      *
      * @return int status
      */
-    public static function extCopyDirectory(Exec $exec, $id, $source, $target)
+    public static function extCopyDirectory(Exec $exec, $id, $source, $target, array $tarFlags = array())
     {
         if ('' === $source) {
             throw new \InvalidArgumentException('empty source');
         }
 
         $cd = Lib::cmd('cd', array($source . '/.'));
-        $tar = Lib::cmd('tar', array('c', '-f', '-', '.'));
+        $tar = Lib::cmd('tar', array('c', '-f', '-', $tarFlags, '.'));
         $dockerCp = Lib::cmd('docker ', array('cp', '-', $id . ':' . $target));
 
         return $exec->pass("{$cd} && {$tar} | {$dockerCp}", array());
@@ -106,16 +109,37 @@ public static function extCopyDirectory(Exec $exec, $id, $source, $target)
      * @param string $id container id
      * @param string $source directory
      * @param string $target directory to create within container
+     * @param array $tarFlags flags for tar, optional
      *
      * @return int
      */
-    public static function extDeployDirectory(Exec $exec, $id, $source, $target)
+    public static function extDeployDirectory(Exec $exec, $id, $source, $target, array $tarFlags = array())
     {
-        $status = self::extMakeEmptyDirectory($exec, $id, $source, $target);
+        $status = self::extMakeEmptyDirectory($exec, $id, $source, $target, $tarFlags);
         if (0 !== $status) {
             return $status;
         }
 
-        return self::extCopyDirectory($exec, $id, $source, $target);
+        return self::extCopyDirectory($exec, $id, $source, $target, $tarFlags);
+    }
+
+    /**
+     * @param null|User $user
+     *
+     * @return array
+     */
+    public static function ownerOpts($user)
+    {
+        if (null === $user) {
+            return array();
+        }
+
+        if (!($user instanceof User)) {
+            throw new \InvalidArgumentException('$user must be a User');
+        }
+
+        list($uid, $gid) = $user->toUidGidArray();
+
+        return array('--numeric-owner', sprintf('--owner=:%d', $uid), sprintf('--group=:%d', $gid));
     }
 }
diff --git a/src/Runner/Opts/User.php b/src/Runner/Opts/User.php
@@ -0,0 +1,77 @@
+<?php
+
+/* this file is part of pipelines */
+
+namespace Ktomk\Pipelines\Runner\Opts;
+
+use Ktomk\Pipelines\Preg;
+
+/**
+ * <name|uid>[:<group|gid>]
+ *
+ * from --user[=<name|uid>[:<group|gid>]]
+ */
+final class User
+{
+    /**
+     * @var string
+     */
+    private $user;
+
+    /**
+     * @param non-empty-string $user
+     *
+     * @return self
+     */
+    public function __construct($user)
+    {
+        $this->user = $user;
+    }
+
+    /**
+     * @return array{0: int, 1: int|null}
+     */
+    public function toUidGidArray()
+    {
+        list($user, $group) = explode(':', $this->user, 2) + array(null, null);
+
+        /* --user=0:0 */
+        if ($this->isId($user) && $this->isId($group)) {
+            return array((int)$user, (int)$group);
+        }
+
+        /* --user=0 */
+        if ($this->isId($user) && null === $group) {
+            return array((int)$user, $group);
+        }
+
+        /* --user=name[:group] */
+        $match = Preg::match('/^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,30}[a-zA-Z0-9_.$-]?$/', $user);
+        if (0 === $match) {
+            throw new \RuntimeException(sprintf('illegal username: %s', addcslashes($user, "\0..\40'\177..\377")));
+        }
+
+        throw new \RuntimeException(sprintf('unable to find user %s: there is no owner and group map. use numeric user/group ids instead.', $user));
+    }
+
+    /**
+     * @return string
+     */
+    public function toString()
+    {
+        return $this->user;
+    }
+
+    /**
+     * @param string $subject
+     *
+     * @return bool
+     */
+    private function isId($subject)
+    {
+        return (
+            $subject === (string)(int)$subject
+            && '-' !== $subject[0]
+        );
+    }
+}
diff --git a/src/Runner/RunOpts.php b/src/Runner/RunOpts.php
@@ -4,6 +4,7 @@
 
 namespace Ktomk\Pipelines\Runner;
 
+use Ktomk\Pipelines\Runner\Opts\User;
 use Ktomk\Pipelines\Utility\Options;
 use Ktomk\Pipelines\Value\Prefix;
 
@@ -208,11 +209,11 @@ public function setNoManual($noManual)
     }
 
     /**
-     * @return null|string
+     * @return null|User
      */
     public function getUser()
     {
-        return $this->user;
+        return $this->user ? new User($this->user) : null;
     }
 
     /**
@@ -222,6 +223,9 @@ public function getUser()
      */
     public function setUser($user)
     {
+        if ($user instanceof User) {
+            $user = $user->toString();
+        }
         $this->user = $user;
     }
 

diff --git a/src/Runner/StepRunner.php b/src/Runner/StepRunner.php
@@ -232,7 +232,8 @@ private function deployCopy($copy, $id, $dir)
 
         $clonePath = $this->runner->getRunOpts()->getOption('step.clone-path');
 
-        $status = TarCopier::extDeployDirectory($exec, $id, $dir, $clonePath);
+        $tarFlags = TarCopier::ownerOpts($this->runner->getRunOpts()->getUser());
+        $status = TarCopier::extDeployDirectory($exec, $id, $dir, $clonePath, $tarFlags);
         if (0 !== $status) {
             return $status;
         }

diff --git a/test/unit/Runner/Docker/Provision/TarCopierTest.php b/test/unit/Runner/Docker/Provision/TarCopierTest.php
@@ -5,6 +5,7 @@
 namespace Ktomk\Pipelines\Runner\Docker\Provision;
 
 use Ktomk\Pipelines\Cli\ExecTester;
+use Ktomk\Pipelines\Runner\Opts\User;
 use Ktomk\Pipelines\TestCase;
 
 /**
@@ -104,4 +105,41 @@ public function testExtDeployDirectoryQuickHappyPathDribble()
         $exec->expect('pass', "~cd /tmp/pipelines-cp\\.[^/]+/\\. && tar c -f - \\./failure | docker  cp - '\\*test-run\\*:/\\.'~", 42);
         self::assertSame(42, TarCopier::extDeployDirectory($exec, '*test-run*', __DIR__, '/failure'));
     }
+
+    /**
+     * @return void
+     * @covers \Ktomk\Pipelines\Runner\Docker\Provision\TarCopier::ownerOpts
+     */
+    public function testOwnerOptsFallthrough()
+    {
+        self::assertSame(array(), TarCopier::ownerOpts(null));
+    }
+
+    /**
+     * @return void
+     * @covers \Ktomk\Pipelines\Runner\Docker\Provision\TarCopier::ownerOpts
+     */
+    public function testOwnerOptsWithNonNullNonUser()
+    {
+        $this->expectException('InvalidArgumentException');
+        $this->expectExceptionMessage('$user must be a User');
+        /** @noinspection PhpParamsInspection intended */
+        TarCopier::ownerOpts(false);
+    }
+
+    /**
+     * @return void
+     * @covers \Ktomk\Pipelines\Runner\Docker\Provision\TarCopier::ownerOpts
+     */
+    public function testOwnerOptsWithUser()
+    {
+        self::assertSame(
+            array(
+                '--numeric-owner',
+                '--owner=:1000',
+                '--group=:1000',
+            ),
+            TarCopier::ownerOpts(new User('1000:1000'))
+        );
+    }
 }
diff --git a/test/unit/Runner/Opts/UserTest.php b/test/unit/Runner/Opts/UserTest.php
@@ -0,0 +1,55 @@
+<?php
+
+/* this file is part of pipelines */
+
+namespace Ktomk\Pipelines\Runner\Opts;
+
+use Ktomk\Pipelines\TestCase;
+
+/**
+ * @covers \Ktomk\Pipelines\Runner\Opts\User
+ */
+class UserTest extends TestCase
+{
+    public function testCreation()
+    {
+        $user = new User('0');
+        self::assertNotNull($user);
+    }
+
+    public function testUserToString()
+    {
+        $user = new User('0');
+        self::assertSame('0', $user->toString());
+    }
+
+    public function testUserToUidGidOptional()
+    {
+        $user = new User('0');
+        $actual = $user->toUidGidArray();
+        self::assertSame(array(0, null), $actual);
+    }
+
+    public function testUserToUidGid()
+    {
+        $user = new User('0:0');
+        $actual = $user->toUidGidArray();
+        self::assertSame(array(0, 0), $actual);
+    }
+
+    public function testUserToUidLetsNotPassMinusInFront()
+    {
+        $user = new User('-0');
+        $this->expectException('RuntimeException');
+        $this->expectExceptionMessage('illegal username: -0');
+        $user->toUidGidArray();
+    }
+
+    public function testUserToUidGidThrowsInUsernameBecauseNoMaps()
+    {
+        $user = new User('uname');
+        $this->expectException('RuntimeException');
+        $this->expectExceptionMessage('unable to find user uname: there is no owner and group map. use numeric user/group ids instead.');
+        $user->toUidGidArray();
+    }
+}
diff --git a/test/unit/Runner/RunOptsTest.php b/test/unit/Runner/RunOptsTest.php
@@ -115,9 +115,20 @@ public function testNoManual(RunOpts $opts)
      */
     public function testUser(RunOpts $opts)
     {
-        self::assertNull($opts->getUser());
         $opts->setUser('foo');
-        self::assertSame('foo', $opts->getUser());
+        self::assertSame('foo', $opts->getUser()->toString());
+    }
+
+    /**
+     * @depends testCreation
+     *
+     * @param RunOpts $opts
+     */
+    public function testUserAsUser(RunOpts $opts)
+    {
+        $opts->setUser('foo');
+        $opts->setUser($opts->getUser());
+        self::assertSame('foo', $opts->getUser()->toString());
     }
 
     /**

diff --git a/test/unit/Utility/RunnerOptionsTest.php b/test/unit/Utility/RunnerOptionsTest.php
@@ -101,7 +101,7 @@ public function testUserOptionSet()
         $exec->expect('capture', '~^printf ~', 0);
 
         $runOpts = $runnerOptions->run();
-        self::assertIsString($runOpts->getUser());
+        self::assertNull($runOpts->getUser());
 
         $exec->expect('capture', '~^printf ~', 1);