Repository: cekit/cekit

Issue:
Remove support for old type repostitories
Triggered by #385. The deprecation message is there for a while, we can safely remove support for it.

```
2019-02-14 09:50:20,632 cekit        WARNING  The way of defining repository 'jboss-os' is deprecated. Convert it to an URL based repository object. Consult Cekit docs, for more details.
```

Old repositories are defined like this:

```yaml
packages:
  repositories:
    - jboss-os
```

Discussion:
rnc: This doesn't need any docs update does it?
goldmann: No, it was already removed from docs some time ago, even from stable.

Patch:
diff --git a/.circleci/config.yml b/.circleci/config.yml
index 3ed99b9..f2bfe90 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -1,6 +1,6 @@
 version: 2
 jobs:
-  build:
+  test:
     parallelism: 4
     machine:
       image: circleci/classic:201808-01
@@ -28,3 +28,32 @@ jobs:
           if [[ "${CIRCLE_NODE_INDEX}" == 3 ]]; then
              make test-py37
           fi
+
+  docs-build:
+    machine:
+      image: circleci/classic:201808-01
+    steps:
+      - checkout
+      - run:
+          name: Install docs dependencies
+          command: |
+            cd docs
+            mkdir -p ./venv
+            virtualenv ./venv
+            . venv/bin/activate
+            pip install -r requirements.txt
+      - run:
+          name: Generate documentation
+          command: |
+            cd docs
+            . venv/bin/activate
+            make html
+      - store_artifacts:
+          path: docs/_build
+
+workflows:
+  version: 2
+  build:
+    jobs:
+      - test
+      - docs-build
diff --git a/cekit/builders/buildah.py b/cekit/builders/buildah.py
index 004e543..111b77b 100644
--- a/cekit/builders/buildah.py
+++ b/cekit/builders/buildah.py
@@ -30,6 +30,9 @@ class BuildahBuilder(Builder):
         tags = self.params.tags
         cmd = ["/usr/bin/buildah", "build-using-dockerfile"]
 
+        if not tags:
+            tags = self.generator.get_tags()
+
         if self.params.pull:
             cmd.append('--pull-always')
 
diff --git a/cekit/builders/podman.py b/cekit/builders/podman.py
index 5ccfdd2..1ac3bbe 100644
--- a/cekit/builders/podman.py
+++ b/cekit/builders/podman.py
@@ -30,6 +30,9 @@ class PodmanBuilder(Builder):
         tags = self.params.tags
         cmd = ["/usr/bin/podman", "build"]
 
+        if not tags:
+            tags = self.generator.get_tags()
+
         if self.params.pull:
             cmd.append('--pull-always')
 
diff --git a/cekit/descriptor/packages.py b/cekit/descriptor/packages.py
index 1c49fa3..a5131fc 100644
--- a/cekit/descriptor/packages.py
+++ b/cekit/descriptor/packages.py
@@ -46,6 +46,7 @@ class Packages(Descriptor):
     Args:
       descriptor - yaml containing Packages section
     """
+
     def __init__(self, descriptor, descriptor_path):
         self.schemas = packages_schema
         self.descriptor_path = descriptor_path
@@ -95,6 +96,7 @@ class Packages(Descriptor):
         self._descriptor['content_sets_file'] = value
         self._descriptor.pop('content_sets', None)
 
+
 class Repository(Descriptor):
     """Object representing package repository
 
@@ -103,13 +105,8 @@ class Repository(Descriptor):
     """
 
     def __init__(self, descriptor):
-        # we test parameter is not dict as there is no easy way how to test
-        # if something is string both in py2 and py3
-        if not isinstance(descriptor, dict):
-            descriptor = self._create_repo_object(descriptor)
-
-        if 'filename' not in descriptor:
-            descriptor['filename'] = '%s.repo' % descriptor['name'].replace(' ', '_')
+        self.schemas = [repository_schema]
+        super(Repository, self).__init__(descriptor)
 
         if not (('url' in descriptor) ^
                 ('odcs' in descriptor) ^
@@ -119,13 +116,12 @@ class Repository(Descriptor):
                              "['id', 'odcs', 'rpm', 'url']"
                              % descriptor['name'])
 
+        if 'filename' not in descriptor:
+            descriptor['filename'] = '%s.repo' % descriptor['name'].replace(' ', '_')
+
         if 'url' not in descriptor:
             descriptor['url'] = {}
 
-
-        self.schemas = [repository_schema]
-        super(Repository, self).__init__(descriptor)
-
         # we dont want to merge any of theese
         self.skip_merging = ['rpm',
                              'id',
@@ -134,46 +130,11 @@ class Repository(Descriptor):
         if 'present' not in self._descriptor:
             self._descriptor['present'] = True
 
-    def _create_repo_object(self, repository):
-        logger.warning("The way of defining repository '%s' is deprecated. Convert "
-                       "it to an URL based repository object. Consult Cekit docs, "
-                       "for more details." % repository)
-        descriptor = {}
-        descriptor['name'] = repository
-        descriptor['url'] = {}
-        descriptor['url']['repository'] = self._get_repo_url(descriptor)
-        return descriptor
-
-    def _get_repo_url(self, descriptor):
-        """Retruns repository url from Cekit config files repositories section"""
-        configured_repositories = config.get('repositories')
-
-        # We need to remove the custom "__name__" element before we can show
-        # which repository keys are defined in the configuration
-        configured_repository_names = configured_repositories.keys()
-
-        if '__name__' in configured_repository_names:
-            configured_repository_names.remove('__name__')
-
-        if descriptor['name'] not in configured_repositories:
-            if len(configured_repository_names):
-                logger.warning("Package repository '%s' used in descriptor is not "
-                                 "available in Cekit configuration file. "
-                                 "Available repositories: %s"
-                                 % (descriptor['name'], ' '.join(configured_repository_names)))
-            else:
-                logger.warning("Package repository '%s' used in descriptor is not "
-                                 "available in Cekit configuration file. "
-                                 % descriptor['name'])
-            return None
-
-        return configured_repositories[descriptor['name']]
-
     def fetch(self, target_dir):
         if not self._descriptor['url']['repository']:
             raise CekitError("Repository not defined for '%s'." % (self.name))
         if not os.path.exists(target_dir):
-                os.makedirs(target_dir)
+            os.makedirs(target_dir)
         Resource({'url': self._descriptor['url']['repository']}) \
             .copy(os.path.join(target_dir, self._descriptor['filename']))
 
diff --git a/cekit/descriptor/resource.py b/cekit/descriptor/resource.py
index b506057..2917776 100644
--- a/cekit/descriptor/resource.py
+++ b/cekit/descriptor/resource.py
@@ -156,16 +156,19 @@ class Resource(Descriptor):
 
     def __substitute_cache_url(self, url):
         cache = config.get('common', 'cache_url')
+
         if not cache:
             return url
 
         for algorithm in SUPPORTED_HASH_ALGORITHMS:
             if algorithm in self:
-                logger.debug("Using %s to fetch artifacts from cacher."
-                             % algorithm)
-                return (cache.replace('#filename#', self.name)
-                        .replace('#algorithm#', algorithm)
-                        .replace('#hash#', self[algorithm]))
+                logger.debug("Using {} checksum to fetch artifacts from cacher".format(algorithm))
+
+                url = cache.replace('#filename#', self.name).replace(
+                    '#algorithm#', algorithm).replace('#hash#', self[algorithm])
+
+                logger.debug("Using cache url '{}'".format(url))
+
         return url
 
     def _download_file(self, url, destination, use_cache=True):
@@ -312,26 +315,30 @@ class _PlainResource(Resource):
         self.target = self.target_file_name()
 
     def _copy_impl(self, target):
-
+        # First of all try to download the file using cacher if specified
         if config.get('common', 'cache_url'):
-            logger.debug("Trying to download artifact %s from remote cache" % self.name)
-            # If cacher URL is set, use it
             try:
-                self._download_file(self.url, target)
+                self._download_file(None, target)
                 return target
-            except:
-                logger.warning("Could not download artifact %s from the remote cache" % self.name)
+            except Exception as e:
+                logger.debug(str(e))
+                logger.warning("Could not download '{}' artifact using cacher".format(self.name))
 
         md5 = self.get('md5')
 
+        # Next option is to download it from Brew directly but only if the md5 checkum
+        # is provided and we are running with the --redhat switch
         if md5 and config.get('common', 'redhat'):
-            logger.debug("Trying to download artifact %s from Brew directly" % self.name)
+            logger.debug("Trying to download artifact '{}' from Brew directly".format(self.name))
 
             try:
-                self.url = get_brew_url(md5)
-                self._download_file(self.url, target, use_cache=False)
+                # Generate the URL
+                url = get_brew_url(md5)
+                # Use the URL to download the file
+                self._download_file(url, target, use_cache=False)
                 return target
-            except:
-                logger.warning("Could not download artifact %s from Brew" % self.name)
+            except Exception as e:
+                logger.debug(str(e))
+                logger.warning("Could not download artifact '{}' from Brew".format(self.name))
 
-        raise CekitError("Artifact %s could not be found" % self.name)
+        raise CekitError("Artifact {} could not be found".format(self.name))
diff --git a/cekit/generator/docker.py b/cekit/generator/docker.py
index 9b00e5b..ec6e701 100644
--- a/cekit/generator/docker.py
+++ b/cekit/generator/docker.py
@@ -48,12 +48,12 @@ class DockerGenerator(Generator):
         arch = platform.machine()
 
         if arch not in content_sets:
-            raise CekitError("There are no contet_sets defined for platform '{}'!".format(arch))
+            raise CekitError("There are no content_sets defined for platform '{}'!".format(arch))
 
         repos = ' '.join(content_sets[arch])
 
         try:
-            # idealy this will be API for ODCS, but there is no python3 package for ODCS
+            # ideally this will be API for ODCS, but there is no python3 package for ODCS
             cmd = ['/usr/bin/odcs']
 
             if config.get('common', 'redhat'):


Test patch:
diff --git a/tests/test_builder.py b/tests/test_builder.py
index 86911b9..d45a40e 100644
--- a/tests/test_builder.py
+++ b/tests/test_builder.py
@@ -7,7 +7,9 @@ import time
 import pytest
 import yaml
 
+from cekit.descriptor import Image
 from cekit.errors import CekitError
+from cekit.generator.docker import DockerGenerator
 from cekit.tools import Map
 
 try:
@@ -572,3 +574,55 @@ def test_podman_builder_run_pull(mocker):
                                         '-t', 'foo',
                                         '-t', 'bar',
                                         'something/image'])
+
+
+def test_podman_builder_run_with_generator(mocker):
+    params = Map({'tags': []})
+    check_call = mocker.patch.object(subprocess, 'check_call')
+    builder = create_builder_object(mocker, 'podman', params)
+    builder.generator = DockerGenerator("", "", {})
+    builder.generator.image = Image(yaml.safe_load("""
+    name: foo
+    version: 1.9
+    labels:
+      - name: test
+        value: val1
+      - name: label2
+        value: val2
+    envs:
+      - name: env1
+        value: env1val
+    """), 'foo')
+    builder.run()
+
+    check_call.assert_called_once_with(['/usr/bin/podman',
+                                        'build',
+                                        '-t', 'foo:1.9',
+                                        '-t', 'foo:latest',
+                                        'something/image'])
+
+
+def test_buildah_builder_run_with_generator(mocker):
+    params = Map({'tags': []})
+    check_call = mocker.patch.object(subprocess, 'check_call')
+    builder = create_builder_object(mocker, 'buildah', params)
+    builder.generator = DockerGenerator("", "", {})
+    builder.generator.image = Image(yaml.safe_load("""
+    name: foo
+    version: 1.9
+    labels:
+      - name: test
+        value: val1
+      - name: label2
+        value: val2
+    envs:
+      - name: env1
+        value: env1val
+    """), 'foo')
+    builder.run()
+
+    check_call.assert_called_once_with(['/usr/bin/buildah',
+                                        'build-using-dockerfile',
+                                        '-t', 'foo:1.9',
+                                        '-t', 'foo:latest',
+                                        'something/image'])
diff --git a/tests/test_unit_descriptor.py b/tests/test_unit_descriptor.py
index d11d571..0aa50fb 100644
--- a/tests/test_unit_descriptor.py
+++ b/tests/test_unit_descriptor.py
@@ -67,20 +67,24 @@ def test_osbs():
     assert osbs['repository']['branch'] == 'bar'
 
 
-def test_packages(mocker):
-    mocker.patch.object(Repository, '_get_repo_url', return_value='foo')
+def test_packages():
     pkg = Packages(yaml.safe_load("""
-      repositories:
-          - repo-foo
-          - repo-bar
       install:
           - pkg-foo"""), "a/path/image.yaml")
 
-    assert Repository('repo-foo') in pkg['repositories']
-    assert Repository('repo-bar') in pkg['repositories']
     assert 'pkg-foo' in pkg['install']
 
 
+def test_packages_invalid_old_repository_definition():
+    with pytest.raises(CekitError, match=r"Cannot validate schema: Repository"):
+        Packages(yaml.safe_load("""
+        repositories:
+            - repo-foo
+            - repo-bar
+        install:
+            - pkg-foo"""), "a/path/image.yaml")
+
+
 def test_image():
     image = Image(yaml.safe_load("""
     from: foo
diff --git a/tests/test_unit_generator_docker.py b/tests/test_unit_generator_docker.py
index b7fc8fe..dbf2d3f 100644
--- a/tests/test_unit_generator_docker.py
+++ b/tests/test_unit_generator_docker.py
@@ -98,7 +98,7 @@ def test_prepare_content_sets_should_fail_when_cs_are_note_defined_for_current_p
 
     with docker_generator(tmpdir) as generator:
         with cekit_config(redhat=True):
-            with pytest.raises(CekitError, match="There are no contet_sets defined for platform 'current_platform'!"):
+            with pytest.raises(CekitError, match="There are no content_sets defined for platform 'current_platform'!"):
                 assert generator._prepare_content_sets(
                     {'invalid_platform': ['ca1', 'cs2']}) is False
 
diff --git a/tests/test_unit_resource.py b/tests/test_unit_resource.py
index b140573..feba92b 100644
--- a/tests/test_unit_resource.py
+++ b/tests/test_unit_resource.py
@@ -70,6 +70,7 @@ def test_fetching_with_ssl_verify(mocker):
     mock_urlopen = get_mock_urlopen(mocker)
 
     res = Resource({'name': 'file', 'url': 'https:///dummy'})
+
     try:
         res.copy()
     except:
@@ -88,6 +89,7 @@ def test_fetching_disable_ssl_verify(mocker):
     get_mock_ssl(mocker, ctx)
 
     res = Resource({'name': 'file', 'url': 'https:///dummy'})
+
     try:
         res.copy()
     except:
@@ -153,7 +155,9 @@ def test_fetching_file_exists_no_hash_fetched_again(mocker):
 
     with open('file', 'w') as f:  # noqa: F841
         pass
+
     res = Resource({'name': 'file', 'url': 'http:///dummy'})
+
     with pytest.raises(CekitError):
         # url is not valid so we get error, but we are not interested
         # in it. We just need to check that we attempted to downlad.
@@ -253,6 +257,63 @@ def test_url_resource_download_cleanup_after_failure(mocker, tmpdir, caplog):
     os_remove_mock.assert_called_with(targetfile)
 
 
+def test_copy_plain_resource_with_cacher(mocker, tmpdir):
+    config.cfg['common']['cache_url'] = '#filename#,#algorithm#,#hash#'
+    config.cfg['common']['work_dir'] = str(tmpdir)
+
+    urlopen_class_mock = mocker.patch('cekit.descriptor.resource.urlopen')
+    mock_urlopen = urlopen_class_mock.return_value
+    mock_urlopen.getcode.return_value = 200
+    mock_urlopen.read.side_effect = [b"one", b"two", None]
+
+    ctx = get_ctx(mocker)
+    get_mock_ssl(mocker, ctx)
+
+    with open('file', 'w') as f:  # noqa: F841
+        pass
+
+    res = Resource({'name': 'foo',
+                    'md5': '5b9164ad6f496d9dee12ec7634ce253f'})
+
+    substitute_cache_url_mock = mocker.patch.object(
+        res, '_Resource__substitute_cache_url', return_value='http://cache/abc')
+
+    res.copy(str(tmpdir))
+
+    substitute_cache_url_mock.assert_called_once_with(None)
+    urlopen_class_mock.assert_called_with('http://cache/abc', context=ctx)
+
+
+def test_copy_plain_resource_from_brew(mocker, tmpdir):
+    config.cfg['common']['work_dir'] = str(tmpdir)
+    config.cfg['common']['redhat'] = True
+
+    urlopen_class_mock = mocker.patch('cekit.descriptor.resource.urlopen')
+    mock_urlopen = urlopen_class_mock.return_value
+    mock_urlopen.getcode.return_value = 200
+    mock_urlopen.read.side_effect = [b"one", b"two", None]
+
+    ctx = get_ctx(mocker)
+    get_mock_ssl(mocker, ctx)
+
+    with open('file', 'w') as f:  # noqa: F841
+        pass
+
+    res = Resource({'name': 'foo',
+                    'md5': '5b9164ad6f496d9dee12ec7634ce253f'})
+
+    mocker.spy(res, '_Resource__substitute_cache_url')
+
+    mock_get_brew_url = mocker.patch(
+        'cekit.descriptor.resource.get_brew_url', return_value='http://cache/abc')
+
+    res.copy(str(tmpdir))
+
+    mock_get_brew_url.assert_called_once_with('5b9164ad6f496d9dee12ec7634ce253f')
+    res._Resource__substitute_cache_url.call_count == 0
+    urlopen_class_mock.assert_called_with('http://cache/abc', context=ctx)
+
+
 def test_overide_resource_remove_chksum():
     image = Image(yaml.safe_load("""
     from: foo
diff --git a/tests/test_validate.py b/tests/test_validate.py
index de50189..b2694b6 100644
--- a/tests/test_validate.py
+++ b/tests/test_validate.py
@@ -107,7 +107,7 @@ def run_cekit_cs_overrides(image_dir, mocker, overrides_descriptor):
 
 
 def test_content_sets_file_container_file(tmpdir, mocker, caplog):
-    # Do not try to validate dependencies while running tests, these are not neccessary
+    # Do not try to validate dependencies while running tests, these are not necessary
     mocker.patch('cekit.generator.docker.DockerGenerator.dependencies').return_value({})
     overrides_descriptor = {
         'schema_version': 1,
@@ -132,7 +132,7 @@ def test_content_sets_file_container_file(tmpdir, mocker, caplog):
 
 
 def test_content_sets_file_container_embedded(tmpdir, mocker, caplog):
-    # Do not try to validate dependencies while running tests, these are not neccessary
+    # Do not try to validate dependencies while running tests, these are not necessary
     mocker.patch('cekit.generator.docker.DockerGenerator.dependencies').return_value({})
     overrides_descriptor = {
         'schema_version': 1,
@@ -153,7 +153,7 @@ def test_content_sets_file_container_embedded(tmpdir, mocker, caplog):
 
 
 def test_content_sets_embedded_container_embedded(tmpdir, mocker, caplog):
-    # Do not try to validate dependencies while running tests, these are not neccessary
+    # Do not try to validate dependencies while running tests, these are not necessary
     mocker.patch('cekit.generator.docker.DockerGenerator.dependencies').return_value({})
     overrides_descriptor = {
         'schema_version': 1,
@@ -169,7 +169,7 @@ def test_content_sets_embedded_container_embedded(tmpdir, mocker, caplog):
 
 
 def test_content_sets_embedded_container_file(tmpdir, mocker, caplog):
-    # Do not try to validate dependencies while running tests, these are not neccessary
+    # Do not try to validate dependencies while running tests, these are not necessary
     mocker.patch('cekit.generator.docker.DockerGenerator.dependencies').return_value({})
     overrides_descriptor = {
         'schema_version': 1,