Repository: cekit/cekit

Issue:
the git "ref:" argument should accept a valid git reference
**Describe the bug**

cekit permits specifying a git ref for a module, but this does not translate into a git ref underneath, but must actually be a tag or branch name (valid argument to `git clone -b`). It would be nicer if this could be a real git ref (e.g. be123a80c5497a595e2a997a0f1dbe14c9e3442e)

**To reproduce**

Attempt to set the git ref for an external module to a ref that is not a branch or tag name.

**Expected behavior**

Remote repository cloned and local branch set up to reference the ref  specified, even if it was a sha1-id.

 - CEKit version: 3.4.0

Discussion:

Patch:
diff --git a/cekit/descriptor/osbs.py b/cekit/descriptor/osbs.py
index dcd23b5..dfee1da 100644
--- a/cekit/descriptor/osbs.py
+++ b/cekit/descriptor/osbs.py
@@ -98,7 +98,6 @@ class Configuration(Descriptor):
         self.schema = configuration_schema
         self.descriptor_path = descriptor_path
         super(Configuration, self).__init__(descriptor)
-        self.skip_merging = ['container', 'container_file']
 
         if 'container' in self and 'container_file' in self:
             raise CekitError('You cannot specify container and container_file together!')
diff --git a/cekit/descriptor/resource.py b/cekit/descriptor/resource.py
index 755cd73..a694c93 100644
--- a/cekit/descriptor/resource.py
+++ b/cekit/descriptor/resource.py
@@ -18,7 +18,7 @@ from cekit.config import Config
 from cekit.crypto import SUPPORTED_HASH_ALGORITHMS, check_sum
 from cekit.descriptor import Descriptor
 from cekit.errors import CekitError
-from cekit.tools import get_brew_url, Map
+from cekit.tools import get_brew_url, Map, Chdir
 
 logger = logging.getLogger('cekit')
 config = Config()
@@ -437,10 +437,15 @@ class _GitResource(Resource):
         return os.path.basename(descriptor.get('git', {}).get('url')).split(".", 1)[0]
 
     def _copy_impl(self, target):
-        cmd = ['git', 'clone', '--depth', '1', self.git.url, target, '-b',
-               self.git.ref]
-        logger.debug("Running '{}'".format(' '.join(cmd)))
+        cmd = ['git', 'clone', self.git.url, target]
+        logger.debug("Cloning Git repository: '{}'".format(' '.join(cmd)))
         subprocess.check_output(cmd, stderr=subprocess.STDOUT)
+
+        with Chdir(target):
+            cmd = ['git', 'checkout', self.git.ref]
+            logger.debug("Checking out '{}' ref: '{}'".format(self.git.ref, ' '.join(cmd)))
+            subprocess.check_output(cmd, stderr=subprocess.STDOUT)
+
         return target
 
 


Test patch:
diff --git a/tests/test_integ_builder_osbs.py b/tests/test_integ_builder_osbs.py
index 09a181f..a717f3d 100644
--- a/tests/test_integ_builder_osbs.py
+++ b/tests/test_integ_builder_osbs.py
@@ -59,10 +59,13 @@ def run_cekit(cwd,
         return result
 
 
-def run_osbs(descriptor, image_dir, mocker, return_code=0, build_command=None):
+def run_osbs(descriptor, image_dir, mocker, return_code=0, build_command=None, general_command=None):
     if build_command is None:
         build_command = ['build', 'osbs']
 
+    if general_command is None:
+        general_command = ['--redhat']
+
     # We are mocking it, so do not require it at test time
     mocker.patch('cekit.builders.osbs.OSBSBuilder.dependencies', return_value={})
     mocker.patch('cekit.builders.osbs.OSBSBuilder._wait_for_osbs_task')
@@ -82,14 +85,10 @@ def run_osbs(descriptor, image_dir, mocker, return_code=0, build_command=None):
         b"UUU"
     ])
 
-    with open(os.path.join(image_dir, 'config'), 'w') as fd:
-        fd.write("[common]\n")
-        fd.write("redhat = True")
-
     with open(os.path.join(image_dir, 'image.yaml'), 'w') as fd:
         yaml.dump(descriptor, fd, default_flow_style=False)
 
-    return run_cekit(image_dir, ['-v',
+    return run_cekit(image_dir, general_command + ['-v',
                                  '--work-dir', image_dir,
                                  '--config', 'config'] + build_command,
                      return_code=return_code)
@@ -521,3 +520,37 @@ def test_osbs_builder_with_fetch_artifacts_file_removal(tmpdir, mocker, caplog):
 
     assert not os.path.exists(os.path.join(str(tmpdir), 'osbs', 'repo', 'fetch-artifacts-url.yaml'))
     assert "Removing old 'fetch-artifacts-url.yaml' file" in caplog.text
+
+
+@pytest.mark.parametrize('flag', [[], ['--redhat']])
+def test_osbs_builder_container_yaml_existence(tmpdir, mocker, caplog, flag):
+    """
+    Make sure that the osbs section is properly merged.
+    The evidence is that the container.yaml file is generated.
+
+    https://github.com/cekit/cekit/issues/631
+    """
+
+    caplog.set_level(logging.DEBUG, logger="cekit")
+
+    mocker.patch('cekit.tools.decision', return_value=True)
+    mocker.patch('cekit.descriptor.resource.urlopen')
+    mocker.patch('cekit.generator.osbs.get_brew_url', return_value='http://random.url/path')
+    mocker.patch.object(subprocess, 'check_output')
+    mocker.patch('cekit.builders.osbs.DistGit.push')
+
+    tmpdir.mkdir('osbs').mkdir('repo')
+
+    with Chdir(os.path.join(str(tmpdir), 'osbs', 'repo')):
+        subprocess.call(["git", "init"])
+        subprocess.call(["touch", "file"])
+        subprocess.call(["git", "add", "file"])
+        subprocess.call(["git", "commit", "-m", "Dummy"])
+
+    descriptor = image_descriptor.copy()
+
+    descriptor["osbs"]["configuration"] = {'container': {'compose': {'pulp_repos': True}}}
+
+    run_osbs(descriptor, str(tmpdir), mocker, general_command=flag)
+
+    assert os.path.exists(os.path.join(str(tmpdir), 'osbs', 'repo', 'container.yaml'))
diff --git a/tests/test_unit_resource.py b/tests/test_unit_resource.py
index 3092044..2a9bd12 100644
--- a/tests/test_unit_resource.py
+++ b/tests/test_unit_resource.py
@@ -7,6 +7,12 @@ from cekit.descriptor import Image, Overrides
 from cekit.descriptor.resource import create_resource
 from cekit.config import Config
 from cekit.errors import CekitError
+from cekit.tools import Chdir
+
+try:
+    from unittest.mock import call
+except ImportError:
+    from mock import call
 
 config = Config()
 
@@ -21,13 +27,18 @@ def setup_function(function):
 def test_repository_dir_is_constructed_properly(mocker):
     mocker.patch('subprocess.check_output')
     mocker.patch('os.path.isdir', ret='True')
+    mocker.patch('cekit.descriptor.resource.Chdir', autospec=True)
+
     res = create_resource({'git': {'url': 'http://host.com/url/repo.git', 'ref': 'ref'}})
+
     assert res.copy('dir') == 'dir/repo'
 
 
 def test_repository_dir_uses_name_if_defined(mocker):
     mocker.patch('subprocess.check_output')
     mocker.patch('os.path.isdir', ret='True')
+    mocker.patch('cekit.descriptor.resource.Chdir', autospec=True)
+
     res = create_resource(
         {'name': 'some-id', 'git': {'url': 'http://host.com/url/repo.git', 'ref': 'ref'}})
     assert res.copy('dir') == 'dir/some-id'
@@ -36,6 +47,8 @@ def test_repository_dir_uses_name_if_defined(mocker):
 def test_repository_dir_uses_target_if_defined(mocker):
     mocker.patch('subprocess.check_output')
     mocker.patch('os.path.isdir', ret='True')
+    mocker.patch('cekit.descriptor.resource.Chdir', autospec=True)
+
     res = create_resource(
         {'target': 'some-name', 'git': {'url': 'http://host.com/url/repo.git', 'ref': 'ref'}})
     assert res.copy('dir') == 'dir/some-name'
@@ -44,17 +57,14 @@ def test_repository_dir_uses_target_if_defined(mocker):
 def test_git_clone(mocker):
     mock = mocker.patch('subprocess.check_output')
     mocker.patch('os.path.isdir', ret='True')
+    mocker.patch('cekit.descriptor.resource.Chdir', autospec=True)
+
     res = create_resource({'git': {'url': 'http://host.com/url/path.git', 'ref': 'ref'}})
     res.copy('dir')
-    mock.assert_called_with(['git',
-                             'clone',
-                             '--depth',
-                             '1',
-                             'http://host.com/url/path.git',
-                             'dir/path',
-                             '-b',
-                             'ref'],
-                            stderr=-2)
+    mock.assert_has_calls([
+        call(['git', 'clone', 'http://host.com/url/path.git', 'dir/path'], stderr=-2),
+        call(['git', 'checkout', 'ref'], stderr=-2)
+    ])
 
 
 def get_res(mocker):