From a83c2b9975700af3036be68eeac3f3cd8251afad Mon Sep 17 00:00:00 2001 From: Thierry Carrez Date: Wed, 30 Nov 2016 15:06:30 +0100 Subject: [PATCH] Validate first tags are on the right branch For the first tag in the series, we skipped the descendant check, which means that an existing SHA from a wrong branch would pass checks. This change adds a branch match check to cover that specific case and emit an error. This change removes a warning which was emitted for first releases, so we adjust the expected warning counts for some impacted tests. Change-Id: I741449a11ea68caeadd5be4aef8d0130efb5f5ec --- openstack_releases/cmds/validate.py | 17 +++++-- openstack_releases/gitutils.py | 20 ++++++++ openstack_releases/tests/test_validate.py | 59 +++++++++++++++++++++-- 3 files changed, 89 insertions(+), 7 deletions(-) diff --git a/openstack_releases/cmds/validate.py b/openstack_releases/cmds/validate.py index 4e690c2842..51071f6376 100644 --- a/openstack_releases/cmds/validate.py +++ b/openstack_releases/cmds/validate.py @@ -261,9 +261,20 @@ def validate_releases(deliverable_info, zuul_layout, 'branch manually') elif not prev_version: - mk_warning('skipping descendant test for ' - 'first release, verify ' - 'branch manually') + # If this is the first version in the series, + # check that the commit is actually on the + # targeted branch. + if not gitutils.check_branch_sha(workdir, + project['repo'], + series_name, + defaults.RELEASE, + project['hash']): + msg = '%s %s not present in %s branch' % ( + project['repo'], + project['hash'], + series_name, + ) + mk_error(msg) else: # Check to see if we are re-tagging the same diff --git a/openstack_releases/gitutils.py b/openstack_releases/gitutils.py index b947baea71..05e2860769 100644 --- a/openstack_releases/gitutils.py +++ b/openstack_releases/gitutils.py @@ -115,6 +115,26 @@ def sha_for_tag(workdir, repo, version): return actual_sha +def check_branch_sha(workdir, repo, series, master, sha): + "Check if the SHA is in the targeted branch." + if series == master: + remote_match = 'master' + else: + remote_match = 'remotes/origin/stable/%s' % series + try: + output = subprocess.check_output( + ['git', 'branch', '-a', '--contains', sha], + cwd=os.path.join(workdir, repo), + ).strip() + for branch in output.split(): + if branch == remote_match: + return True + return False + except subprocess.CalledProcessError as e: + print('ERROR checking SHA on branch: %s [%s]' % (e, e.output.strip())) + return False + + def check_ancestry(workdir, repo, old_version, sha): "Check if the SHA is in the ancestry of the previous version." try: diff --git a/openstack_releases/tests/test_validate.py b/openstack_releases/tests/test_validate.py index 1a16be56f3..505bdd82a2 100644 --- a/openstack_releases/tests/test_validate.py +++ b/openstack_releases/tests/test_validate.py @@ -21,6 +21,7 @@ import mock from oslotest import base import yaml +from openstack_releases import defaults from openstack_releases.cmds import validate @@ -416,8 +417,8 @@ class TestValidateReleases(base.BaseTestCase): warnings.append, errors.append, ) - self.assertEqual(1, len(warnings)) - self.assertEqual(1, len(errors)) + self.assertEqual(0, len(warnings)) + self.assertEqual(2, len(errors)) def test_mismatch_existing(self): deliverable_info = { @@ -444,6 +445,56 @@ class TestValidateReleases(base.BaseTestCase): self.assertEqual(0, len(warnings)) self.assertEqual(1, len(errors)) + def test_hash_from_master_used_in_stable_release(self): + deliverable_info = { + 'artifact-link-mode': 'none', + 'releases': [ + {'version': '1.4.1', + 'projects': [ + {'repo': 'openstack/automaton', + # hash from master + 'hash': 'ec62e6270dba8f3d6a60600876be8fd99f7c5b08'}, + ]} + ], + } + warnings = [] + errors = [] + validate.validate_releases( + deliverable_info, + {'validate-projects-by-name': {}}, + 'newton', + self.tmpdir, + warnings.append, + errors.append, + ) + self.assertEqual(0, len(warnings)) + self.assertEqual(1, len(errors)) + + def test_hash_from_stable_used_in_master_release(self): + deliverable_info = { + 'artifact-link-mode': 'none', + 'releases': [ + {'version': '99.5.0', + 'projects': [ + {'repo': 'openstack/automaton', + # hash from stable/newton + 'hash': '95db03ed96dcd1a582936b4660b4db55ce606e49'}, + ]} + ], + } + warnings = [] + errors = [] + validate.validate_releases( + deliverable_info, + {'validate-projects-by-name': {}}, + defaults.RELEASE, + self.tmpdir, + warnings.append, + errors.append, + ) + self.assertEqual(0, len(warnings)) + self.assertEqual(1, len(errors)) + def test_not_descendent(self): deliverable_info = { 'artifact-link-mode': 'none', @@ -500,7 +551,7 @@ class TestValidateReleases(base.BaseTestCase): warnings.append, errors.append, ) - self.assertEqual(1, len(warnings)) + self.assertEqual(0, len(warnings)) self.assertEqual(1, len(errors)) @mock.patch('openstack_releases.versionutils.validate_version') @@ -531,7 +582,7 @@ class TestValidateReleases(base.BaseTestCase): warnings.append, errors.append, ) - self.assertEqual(1, len(warnings)) + self.assertEqual(0, len(warnings)) self.assertEqual(1, len(errors))