ci-fairy: minio: use the host in the minio:// URL

This allows to login to more than one server at a time and makes
the URL cleaner: we know where we pull/put things from/to.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>

tools/ci_fairy.py

@@ -229,7 +229,7 @@ class S3(object):
         '''
         Factory method to get an S3 object based on
         its path.
-        :param full_path: the full path of the object (local posix path or "minio://bucket/key")
+        :param full_path: the full path of the object (local posix path or "minio://host/bucket/key")
         :type full_path: str
         :param credentials: a file path with the appropriate credentials (access key, secret key and session token)
         :type credentials: str
@@ -241,11 +241,11 @@ class S3(object):
         if not full_path.startswith(prefix):
             return S3Object(full_path)
 
-        # full_path should now be in the form `minio://bucket/key`
+        # full_path should now be in the form `minio://host/bucket/key`
 
         path_str = full_path[len(prefix):]
 
-        s3_remote = S3Remote(credentials)
+        s3_remote = S3Remote(path_str, credentials)
 
         return s3_remote.get(path_str)
 
@@ -299,11 +299,29 @@ class S3Remote(S3Object):
     A remote S3 server.
     This contains the list of buckets.
     '''
-    def __init__(self, credentials):
+    def __init__(self, full_path, credentials):
         super().__init__('/')
 
         with open(credentials) as credfile:
-            creds = json.load(credfile)
+            data = json.load(credfile)
+
+        host = full_path
+
+        if '/' in full_path:
+            host, _ = full_path.split('/', 1)
+
+        creds = None
+
+        for h, c in data.items():
+            if h == host:
+                creds = c
+                break
+
+        if not host:
+            raise KeyError(f"missing host information in 'minio://{full_path}'")
+
+        if creds is None:
+            raise KeyError(f"host '{host}' not found in credentials, please use 'ci-fairy minio login' first")
 
         s3 = boto3.resource('s3',
                             endpoint_url=creds['endpoint_url'],
@@ -312,7 +330,7 @@ class S3Remote(S3Object):
                             aws_session_token=creds['SessionToken'],
                             config=Config(signature_version='s3v4'),
                             region_name='us-east-1')
-        self._name = creds['endpoint_url']
+        self._name = host
         self.bucket_names = [b.name for b in s3.buckets.all()]
         self.buckets = {b: s3.Bucket(b) for b in self.bucket_names}
 
@@ -340,8 +358,11 @@ class S3Remote(S3Object):
         return [S3Bucket(b) for b in self.buckets.values()]
 
     def get(self, path):
+        # Remove the host from the URL
+        path = path[len(self.name):].lstrip('/')
+
         if not path:
-            # minio://
+            # minio://host/
             return self
 
         bucket_name = path
@@ -353,7 +374,7 @@ class S3Remote(S3Object):
         try:
             bucket = self.buckets[bucket_name]
         except KeyError:
-            # minio://bucket_that_doesn_t_exist
+            # minio://host/bucket_that_doesn_t_exist
             raise FileNotFoundError(f"bucket '{bucket_name}' doesn't exist on {self.name}")
 
         return S3Bucket(bucket).get(key)
@@ -386,14 +407,14 @@ class S3Bucket(S3Object):
 
         for o in objs:
             if '/' in o:
-                # minio://bucket/some/path/some/file
+                # minio://host/bucket/some/path/some/file
                 # o is now: some/path/some/file
                 root, _ = o.split('/', 1)
                 if root not in children:
                     # root is "some" and is not in the children list
                     children.append(root)
             else:
-                # minio://bucket/some_file
+                # minio://host/bucket/some_file
                 children.append(o)
 
         return [S3RemoteObject(self, c) for c in children]
@@ -404,8 +425,8 @@ class S3Bucket(S3Object):
 
     def get(self, key):
         if not key:
-            # - minio://bucket
-            # - minio://bucket/
+            # - minio://host/bucket
+            # - minio://host/bucket/
             return self
 
         return S3RemoteObject(self, key)
@@ -593,6 +614,15 @@ def minio():
 @click.argument('token')
 def login(credentials, endpoint_url, token):
     '''Login to the minio server'''
+    from urllib.parse import urlparse
+
+    credentials = Path(credentials)
+
+    data = {}
+    if credentials.exists():
+        with open(credentials) as infile:
+            data = json.load(infile)
+
     session = boto3.Session()
 
     sts = session.client('sts',
@@ -605,11 +635,15 @@ def login(credentials, endpoint_url, token):
                                             WebIdentityToken=token,
                                             RoleArn=roleArn,
                                             RoleSessionName='session_name')
+
+    server = urlparse(endpoint_url).netloc
+    server = server.strip('/')
     creds = ret['Credentials']
     creds['endpoint_url'] = endpoint_url
     creds['Expiration'] = creds['Expiration'].isoformat()
+    data[server] = creds
     with open(credentials, 'w') as outfile:
-        json.dump(creds, outfile)
+        json.dump(data, outfile)
 
 
 @minio.command()
@@ -621,6 +655,8 @@ def ls(ctx, credentials, path):
         s3_obj = S3.s3(path, credentials)
     except FileNotFoundError as e:
         ctx.fail(e)
+    except KeyError as e:
+        ctx.fail(e)
 
     if not s3_obj.exists:
         ctx.fail(f"file '{path}' does not exist")
@@ -642,6 +678,8 @@ def cp(ctx, credentials, src, dst):
         src = S3.s3(src, credentials)
     except FileNotFoundError as e:
         ctx.fail(e)
+    except KeyError as e:
+        ctx.fail(e)
 
     # src doesn't exist
     if not src.exists:
@@ -651,6 +689,8 @@ def cp(ctx, credentials, src, dst):
         dst = S3.s3(dst, credentials)
     except FileNotFoundError as e:
         ctx.fail(e)
+    except KeyError as e:
+        ctx.fail(e)
 
     try:
         dst.copy_from(src)

tools/test_ci_fairy.py

@@ -13,6 +13,7 @@ GITLAB_TEST_URL = 'https://test.gitlab.url'
 GITLAB_TEST_PROJECT_ID = '11'
 GITLAB_TEST_PROJECT_PATH = 'project12/path34'
 
+MINIO_TEST_SERVER = 'min.io.url:9000'
 MINIO_TEST_URL = 'http://min.io.url:9000'
 
 # A note on @patch('ci_fairy.Gitlab')
@@ -868,10 +869,12 @@ def mock_minio(minio):
 def write_minio_credentials():
     with open('.minio_credentials', 'w') as f:
         json.dump({
-            'endpoint_url': MINIO_TEST_URL,
-            'AccessKeyId': '1234',
-            'SecretAccessKey': '5678',
-            'SessionToken': '9101112'
+            MINIO_TEST_SERVER: {
+                'endpoint_url': MINIO_TEST_URL,
+                'AccessKeyId': '1234',
+                'SecretAccessKey': '5678',
+                'SessionToken': '9101112',
+            }
         }, f)
 
 
@@ -881,14 +884,18 @@ def write_minio_credentials():
     ('.', ['hello.txt', '.minio_credentials']),
     ('minio:', None),
     ('minio:/', None),
-    ('minio://', [f'bucket{i}' for i in range(3)]),
-    ('minio://bucket1', ['root_file.txt'] + [f'dir-{i}' for i in range(5)]),
-    ('minio://bucket0/', ['root_file.txt'] + [f'dir-{i}' for i in range(5)]),
-    ('minio://non_existant_bucket', None),
-    ('minio://bucket0/non_existent_dir_or_file', None),
-    ('minio://bucket2/dir-2', [f'file-{i}' for i in range(4)]),
-    ('minio://bucket0/dir-1/', [f'file-{i}' for i in range(4)]),
-    ('minio://bucket2/dir-0/file-3', ['file-3']),
+    ('minio://', None),
+    (f'minio://{MINIO_TEST_SERVER}', [f'bucket{i}' for i in range(3)]),
+    (f'minio://{MINIO_TEST_SERVER}/', [f'bucket{i}' for i in range(3)]),
+    ('minio://WRONG_MINIO_TEST_SERVER', None),
+    ('minio://WRONG_MINIO_TEST_SERVER/bucket/path', None),
+    (f'minio://{MINIO_TEST_SERVER}/bucket1', ['root_file.txt'] + [f'dir-{i}' for i in range(5)]),
+    (f'minio://{MINIO_TEST_SERVER}/bucket0/', ['root_file.txt'] + [f'dir-{i}' for i in range(5)]),
+    (f'minio://{MINIO_TEST_SERVER}/non_existant_bucket', None),
+    (f'minio://{MINIO_TEST_SERVER}/bucket0/non_existent_dir_or_file', None),
+    (f'minio://{MINIO_TEST_SERVER}/bucket2/dir-2', [f'file-{i}' for i in range(4)]),
+    (f'minio://{MINIO_TEST_SERVER}/bucket0/dir-1/', [f'file-{i}' for i in range(4)]),
+    (f'minio://{MINIO_TEST_SERVER}/bucket2/dir-0/file-3', ['file-3']),
 ])
 def test_minio_ls(minio, input_path, result_files, caplog):
     runner = CliRunner()
@@ -912,10 +919,16 @@ def test_minio_ls(minio, input_path, result_files, caplog):
             assert result.exit_code == 2
             prefix = 'minio://'
             error_msg = result.output.strip().split('\n')[-1]
+
             if input_path.startswith(prefix):
-                assert input_path[len(prefix):] in error_msg
-            else:
-                assert input_path in error_msg
+                input_path = input_path[len(prefix):]
+
+            if input_path.startswith('WRONG_MINIO_TEST_SERVER'):
+                input_path = 'WRONG_MINIO_TEST_SERVER'
+            elif input_path.startswith(MINIO_TEST_SERVER):
+                input_path = MINIO_TEST_SERVER
+
+            assert input_path in error_msg
 
         else:
             assert result.exit_code == 0
@@ -957,43 +970,43 @@ def test_minio_ls_no_creds(minio, caplog):
     (['hello.txt', 'emptydir/an_other_local_file'], None),
 
     # download an existing distant file
-    (['minio://bucket0/dir-1/file-2', 'result'], None),
+    ([f'minio://{MINIO_TEST_SERVER}/bucket0/dir-1/file-2', 'result'], None),
 
     # download an existing distant file on an existing dir
-    (['minio://bucket0/dir-1/file-2', 'emptydir/'], None),
+    ([f'minio://{MINIO_TEST_SERVER}/bucket0/dir-1/file-2', 'emptydir/'], None),
 
     # download an existing distant file on an existing dir
-    (['minio://bucket0/dir-1/file-2', 'emptydir/result'], None),
+    ([f'minio://{MINIO_TEST_SERVER}/bucket0/dir-1/file-2', 'emptydir/result'], None),
 
     # download an existing distant file on an existing dir
-    (['minio://bucket1/dir-2/file-3', 'emptydir//result'], None),
+    ([f'minio://{MINIO_TEST_SERVER}/bucket1/dir-2/file-3', 'emptydir//result'], None),
 
     # download an existing distant file on an existing dir
-    (['minio://bucket2/dir-0/file-2', 'emptydir///result'], None),
+    ([f'minio://{MINIO_TEST_SERVER}/bucket2/dir-0/file-2', 'emptydir///result'], None),
 
     # upload a local file on an existing bucket
-    (['hello.txt', 'minio://bucket0/'], None),
+    (['hello.txt', f'minio://{MINIO_TEST_SERVER}/bucket0/'], None),
 
     # upload a local file on an existing bucket
-    (['hello.txt', 'minio://bucket0/hello2.txt'], None),
+    (['hello.txt', f'minio://{MINIO_TEST_SERVER}/bucket0/hello2.txt'], None),
 
     # upload a local file on a new dir on an existing bucket
-    (['hello.txt', 'minio://bucket1/new_dir/'], None),
+    (['hello.txt', f'minio://{MINIO_TEST_SERVER}/bucket1/new_dir/'], None),
 
     # upload a local file on a new dir on an existing bucket
-    (['hello.txt', 'minio://bucket0/new_dir/hello2.txt'], None),
+    (['hello.txt', f'minio://{MINIO_TEST_SERVER}/bucket0/new_dir/hello2.txt'], None),
 
     # upload a local file on an existing dir on an existing bucket
-    (['hello.txt', 'minio://bucket0/dir-1/'], None),
+    (['hello.txt', f'minio://{MINIO_TEST_SERVER}/bucket0/dir-1/'], None),
 
     # upload a local file on an existing dir on an existing bucket
-    (['hello.txt', 'minio://bucket0/dir-1/hello2.txt'], None),
+    (['hello.txt', f'minio://{MINIO_TEST_SERVER}/bucket0/dir-1/hello2.txt'], None),
 
     # upload a local file on an existing dir on an existing bucket
-    (['emptydir/../hello.txt', 'minio://bucket2/dir-3/'], None),
+    (['emptydir/../hello.txt', f'minio://{MINIO_TEST_SERVER}/bucket2/dir-3/'], None),
 
     # upload a local file on an existing dir on an existing bucket
-    (['hello.txt', 'minio://bucket0//dir-1/hello2.txt'], None),
+    (['hello.txt', f'minio://{MINIO_TEST_SERVER}/bucket0//dir-1/hello2.txt'], None),
 
     #######################################################
     # failures
@@ -1012,40 +1025,46 @@ def test_minio_ls_no_creds(minio, caplog):
     (['local_file', 'an_other_local_file'], "Error: source file 'local_file' does not exist"),
 
     # source file does not exist
-    (['minio://bucket_that_does_not_exist/file', 'an_other_local_file'], "Error: bucket 'bucket_that_does_not_exist' doesn't exist"),
+    ([f'minio://{MINIO_TEST_SERVER}/bucket_that_does_not_exist/file', 'an_other_local_file'], "Error: bucket 'bucket_that_does_not_exist' doesn't exist"),
 
     # source file does not exist
-    (['minio://bucket1/file', 'an_other_local_file'], "Error: source file 'file' does not exist"),
+    ([f'minio://{MINIO_TEST_SERVER}/bucket1/file', 'an_other_local_file'], "Error: source file 'file' does not exist"),
 
     # source file is a local dir
     (['.', 'local_file'], 'Error: cannot do recursive cp'),
 
     # source file is a remote dir
-    (['minio://bucket0/', 'local_file'], 'Error: cannot do recursive cp'),
+    ([f'minio://{MINIO_TEST_SERVER}/bucket0/', 'local_file'], 'Error: cannot do recursive cp'),
 
     # source file is a remote dir
-    (['minio://bucket1/dir-0', 'local_file'], 'Error: cannot do recursive cp'),
+    ([f'minio://{MINIO_TEST_SERVER}/bucket1/dir-0', 'local_file'], 'Error: cannot do recursive cp'),
 
     # source file is a remote dir
-    (['minio://bucket1/dir-1/', 'local_file'], 'Error: cannot do recursive cp'),
+    ([f'minio://{MINIO_TEST_SERVER}/bucket1/dir-1/', 'local_file'], 'Error: cannot do recursive cp'),
 
     # source file is a local empty dir
     (['emptydir', 'local_file'], 'Error: cannot do recursive cp'),
 
     # source and destination file are remote
-    (['minio://bucket1/dir-0/file-3', 'minio://bucket2/an_other_local_file'], 'Error: at least one argument must be a local path'),
+    ([f'minio://{MINIO_TEST_SERVER}/bucket1/dir-0/file-3', f'minio://{MINIO_TEST_SERVER}/bucket2/an_other_local_file'], 'Error: at least one argument must be a local path'),
+
+    # destination is invalid (no host)
+    (['hello.txt', 'minio://'], 'Error: "missing host information in \'minio://\'"'),
 
     # destination is invalid (no bucket)
-    (['hello.txt', 'minio://'], 'Error: No destination bucket provided'),
+    (['hello.txt', f'minio://{MINIO_TEST_SERVER}/'], 'Error: No destination bucket provided'),
+
+    # destination is invalid (wrong host)
+    (['hello.txt', 'minio://WRONG_MINIO_TEST_SERVER/'], 'Error: "host \'WRONG_MINIO_TEST_SERVER\' not found in credentials'),
 
     # destination is invalid (no bucket)
-    (['hello.txt', 'minio:///dir-0/'], "Error: bucket '' doesn't exist"),
+    (['hello.txt', f'minio://{MINIO_TEST_SERVER}//dir-0/'], "Error: bucket 'dir-0' doesn't exist"),
 
     # destination is invalid (wrong bucket)
-    (['hello.txt', 'minio://this_bucket_doesnt_exist/dir-0/'], "Error: bucket 'this_bucket_doesnt_exist' doesn't exist"),
+    (['hello.txt', f'minio://{MINIO_TEST_SERVER}/this_bucket_doesnt_exist/dir-0/'], "Error: bucket 'this_bucket_doesnt_exist' doesn't exist"),
 
     # destination is invalid (new dir)
-    (['minio://bucket1/dir-0/file-1', 'new_dir/'], "Error: directory 'new_dir' does not exist"),
+    ([f'minio://{MINIO_TEST_SERVER}/bucket1/dir-0/file-1', 'new_dir/'], "Error: directory 'new_dir' does not exist"),
 ])
 def test_minio_cp(minio, input_args, expected_error, caplog):
     runner = CliRunner()