summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFelipe Monteiro <felipe.monteiro@att.com>2018-10-29 19:27:06 +0000
committerTin Lam <tin@irrational.io>2019-01-01 19:17:11 +0000
commit40da373023c508f216a4246ee04efe5ba4bd05de (patch)
tree5d2ab11409b9dd8a8bff9257916a9e112276f1ca
parenta019d131d1fe4fd0b3b493c4a7699954f53b2471 (diff)
tests: Increase test coverage for lint checks
This patch set expands on the unit test coverage for lint checks in test_selectable_linting which only covers a small subset of the lint checks handled by Pegleg. This logic should be properly tested as linting is fundamental to Pegleg functionality. Change-Id: I6a59295982abd22bba8036827cefd4186b68e2fb
Notes
Notes (review): Code-Review+2: Aaron Sheffield <ajs@sheffieldfamily.net> Code-Review+1: Lev Morgan <morgan.lev@gmail.com> Workflow+1: Felipe Monteiro <felipe.monteiro@att.com> Code-Review+2: Felipe Monteiro <felipe.monteiro@att.com> Verified+2: Zuul Submitted-by: Zuul Submitted-at: Wed, 02 Jan 2019 15:39:23 +0000 Reviewed-on: https://review.openstack.org/614015 Project: openstack/airship-pegleg Branch: refs/heads/master
-rw-r--r--pegleg/cli.py3
-rw-r--r--pegleg/engine/errorcodes.py11
-rw-r--r--pegleg/engine/exceptions.py8
-rw-r--r--pegleg/engine/util/definition.py5
-rw-r--r--tests/unit/engine/test_selectable_linting.py159
-rw-r--r--tests/unit/test_utils.py4
6 files changed, 166 insertions, 24 deletions
diff --git a/pegleg/cli.py b/pegleg/cli.py
index c7b1019..6a03a70 100644
--- a/pegleg/cli.py
+++ b/pegleg/cli.py
@@ -97,8 +97,7 @@ ALLOW_MISSING_SUBSTITUTIONS_OPTION = click.option(
97 type=click.BOOL, 97 type=click.BOOL,
98 default=True, 98 default=True,
99 help=("Raise Deckhand exception on missing substition sources. " 99 help=("Raise Deckhand exception on missing substition sources. "
100 "Defaults to True.") 100 "Defaults to True."))
101)
102 101
103EXCLUDE_LINT_OPTION = click.option( 102EXCLUDE_LINT_OPTION = click.option(
104 '-x', 103 '-x',
diff --git a/pegleg/engine/errorcodes.py b/pegleg/engine/errorcodes.py
index e4f281e..08261cb 100644
--- a/pegleg/engine/errorcodes.py
+++ b/pegleg/engine/errorcodes.py
@@ -20,3 +20,14 @@ FILE_MISSING_YAML_DOCUMENT_HEADER = 'P006'
20FILE_CONTAINS_INVALID_YAML = 'P007' 20FILE_CONTAINS_INVALID_YAML = 'P007'
21DOCUMENT_LAYER_MISMATCH = 'P008' 21DOCUMENT_LAYER_MISMATCH = 'P008'
22SECRET_NOT_ENCRYPTED_POLICY = 'P009' 22SECRET_NOT_ENCRYPTED_POLICY = 'P009'
23
24ALL_CODES = (
25 SCHEMA_STORAGE_POLICY_MISMATCH_FLAG,
26 REPOS_MISSING_DIRECTORIES_FLAG,
27 DECKHAND_DUPLICATE_SCHEMA,
28 DECKHAND_RENDER_EXCEPTION,
29 FILE_MISSING_YAML_DOCUMENT_HEADER,
30 FILE_CONTAINS_INVALID_YAML,
31 DOCUMENT_LAYER_MISMATCH,
32 SECRET_NOT_ENCRYPTED_POLICY,
33)
diff --git a/pegleg/engine/exceptions.py b/pegleg/engine/exceptions.py
index 8fc8e0e..6ad7892 100644
--- a/pegleg/engine/exceptions.py
+++ b/pegleg/engine/exceptions.py
@@ -14,12 +14,8 @@
14 14
15import logging 15import logging
16 16
17__all__ = ('PeglegBaseException', 17__all__ = ('PeglegBaseException', 'GitException', 'GitAuthException',
18 'GitException', 18 'GitProxyException', 'GitSSHException', 'GitConfigException',
19 'GitAuthException',
20 'GitProxyException',
21 'GitSSHException',
22 'GitConfigException',
23 'GitInvalidRepoException') 19 'GitInvalidRepoException')
24 20
25LOG = logging.getLogger(__name__) 21LOG = logging.getLogger(__name__)
diff --git a/pegleg/engine/util/definition.py b/pegleg/engine/util/definition.py
index f28705a..8ef67f3 100644
--- a/pegleg/engine/util/definition.py
+++ b/pegleg/engine/util/definition.py
@@ -63,9 +63,8 @@ def pluck(site_definition, key):
63 return site_definition['data'][key] 63 return site_definition['data'][key]
64 except Exception as e: 64 except Exception as e:
65 site_name = site_definition.get('metadata', {}).get('name') 65 site_name = site_definition.get('metadata', {}).get('name')
66 raise click.ClickException( 66 raise click.ClickException('failed to get "%s" from site definition '
67 'failed to get "%s" from site definition ' 67 '"%s": %s' % (key, site_name, e))
68 '"%s": %s' % (key, site_name, e))
69 68
70 69
71def site_files(site_name): 70def site_files(site_name):
diff --git a/tests/unit/engine/test_selectable_linting.py b/tests/unit/engine/test_selectable_linting.py
index fb8c26c..f1503b4 100644
--- a/tests/unit/engine/test_selectable_linting.py
+++ b/tests/unit/engine/test_selectable_linting.py
@@ -12,11 +12,17 @@
12# See the License for the specific language governing permissions and 12# See the License for the specific language governing permissions and
13# limitations under the License. 13# limitations under the License.
14 14
15import os
16
15import click 17import click
16import mock 18import mock
17import pytest 19import pytest
18 20
21from deckhand.engine import layering
22from deckhand import errors as dh_errors
23
19from pegleg import config 24from pegleg import config
25from pegleg.engine import errorcodes
20from pegleg.engine import lint 26from pegleg.engine import lint
21from tests.unit.fixtures import create_tmp_deployment_files 27from tests.unit.fixtures import create_tmp_deployment_files
22 28
@@ -28,11 +34,25 @@ For more information, see: https://storyboard.openstack.org/#!/story/2003762
28 34
29 35
30class TestSelectableLinting(object): 36class TestSelectableLinting(object):
37 def setup(self):
38 self.site_yaml_path = os.path.join(os.getcwd(), 'site_yamls')
39
40 def _exclude_all(self, except_code):
41 """Helper to generate list of all error codes to exclude except for
42 ``except_code`` in order to isolate tests that focus on validating
43 ``warn_lint``: Just check that the expected code issues a warning and
44 effectively ignore all other errors.
45 """
46 return [code for code in errorcodes.ALL_CODES if code != except_code]
47
31 @mock.patch.object(lint, '_verify_deckhand_render', return_value=[]) 48 @mock.patch.object(lint, '_verify_deckhand_render', return_value=[])
32 @mock.patch.object(lint, '_verify_no_unexpected_files', return_value=[]) 49 @mock.patch.object(lint, '_verify_no_unexpected_files', return_value=[])
33 def test_lint_excludes_P001(*args): 50 def test_lint_excludes_P001(self, *args):
51 """Test exclude flag for P001 - Document has storagePolicy cleartext
52 (expected is encrypted) yet its schema is a mandatory encrypted type.
53 """
34 exclude_lint = ['P001'] 54 exclude_lint = ['P001']
35 config.set_site_repo('../pegleg/site_yamls/') 55 config.set_site_repo(self.site_yaml_path)
36 56
37 code_1 = 'X001' 57 code_1 = 'X001'
38 msg_1 = 'is a secret, but has unexpected storagePolicy: "cleartext"' 58 msg_1 = 'is a secret, but has unexpected storagePolicy: "cleartext"'
@@ -44,26 +64,34 @@ class TestSelectableLinting(object):
44 lint, '_verify_file_contents', 64 lint, '_verify_file_contents',
45 return_value=msgs) as mock_methed: 65 return_value=msgs) as mock_methed:
46 with pytest.raises(click.ClickException) as expected_exc: 66 with pytest.raises(click.ClickException) as expected_exc:
47 results = lint.full(False, exclude_lint, []) 67 lint.full(False, exclude_lint, [])
48 assert msg_1 in expected_exc 68 assert msg_1 in expected_exc
49 assert msg_2 in expected_exc 69 assert msg_2 in expected_exc
50 70
51 @pytest.mark.skip(reason=_SKIP_P003_REASON) 71 @pytest.mark.skip(reason=_SKIP_P003_REASON)
52 @mock.patch.object(lint, '_verify_deckhand_render', return_value=[]) 72 @mock.patch.object(lint, '_verify_deckhand_render', return_value=[])
53 def test_lint_excludes_P003(*args): 73 def test_lint_excludes_P003(self, *args):
74 """Test exclude flag for P003 - All repos contain expected
75 directories.
76 """
54 exclude_lint = ['P003'] 77 exclude_lint = ['P003']
55 with mock.patch.object( 78 with mock.patch.object(
56 lint, 79 lint,
57 '_verify_no_unexpected_files', 80 '_verify_no_unexpected_files',
58 return_value=[('P003', 'test message')]) as mock_method: 81 return_value=[('P003', 'test message')]) as mock_method:
59 lint.full(False, exclude_lint, []) 82 result = lint.full(False, exclude_lint, [])
60 mock_method.assert_called() 83 mock_method.assert_called()
84 assert not result # Exclude doesn't return anything.
61 85
62 @mock.patch.object(lint, '_verify_deckhand_render', return_value=[]) 86 @mock.patch.object(lint, '_verify_deckhand_render', return_value=[])
63 @mock.patch.object(lint, '_verify_no_unexpected_files', return_value=[]) 87 @mock.patch.object(lint, '_verify_no_unexpected_files', return_value=[])
64 def test_lint_warns_P001(*args): 88 def test_lint_warns_P001(self, *args):
89 """Test lint flag for P001 - Document has storagePolicy cleartext
90 (expected is encrypted) yet its schema is a mandatory encrypted type.
91 """
92 exclude_lint = self._exclude_all(except_code='P001')
65 warn_lint = ['P001'] 93 warn_lint = ['P001']
66 config.set_site_repo('../pegleg/site_yamls/') 94 config.set_site_repo(self.site_yaml_path)
67 95
68 code_1 = 'X001' 96 code_1 = 'X001'
69 msg_1 = 'is a secret, but has unexpected storagePolicy: "cleartext"' 97 msg_1 = 'is a secret, but has unexpected storagePolicy: "cleartext"'
@@ -75,20 +103,129 @@ class TestSelectableLinting(object):
75 lint, '_verify_file_contents', 103 lint, '_verify_file_contents',
76 return_value=msgs) as mock_methed: 104 return_value=msgs) as mock_methed:
77 with pytest.raises(click.ClickException) as expected_exc: 105 with pytest.raises(click.ClickException) as expected_exc:
78 lint.full(False, [], warn_lint) 106 lint.full(
107 False, exclude_lint=exclude_lint, warn_lint=warn_lint)
79 assert msg_1 not in expected_exc 108 assert msg_1 not in expected_exc
80 assert msg_2 in expected_exc 109 assert msg_2 in expected_exc
81 110
82 @pytest.mark.skip(reason=_SKIP_P003_REASON) 111 @pytest.mark.skip(reason=_SKIP_P003_REASON)
83 @mock.patch.object(lint, '_verify_deckhand_render', return_value=[]) 112 @mock.patch.object(lint, '_verify_deckhand_render', return_value=[])
84 def test_lint_warns_P003(*args): 113 def test_lint_warns_P003(self, *args):
114 """Test warn flag for P003 - All repos contain expected directories."""
115 exclude_lint = self._exclude_all(except_code='P003')
85 warn_lint = ['P003'] 116 warn_lint = ['P003']
86 config.set_site_repo('../pegleg/site_yamls/') 117 config.set_site_repo(self.site_yaml_path)
87 118
88 with mock.patch.object(lint, 119 with mock.patch.object(lint,
89 '_verify_no_unexpected_files') as mock_method: 120 '_verify_no_unexpected_files') as mock_method:
90 lint.full(False, [], warn_lint) 121 result = lint.full(
122 False, exclude_lint=exclude_lint, warn_lint=warn_lint)
91 mock_method.assert_called() 123 mock_method.assert_called()
124 assert len(result) == 1
125 assert result[0].startswith(warn_lint[0])
126
127 @mock.patch('pegleg.engine.util.deckhand.layering', autospec=True)
128 def test_lint_warns_P004(self, mock_layering):
129 """Test warn flag for P004 - Duplicate Deckhand DataSchema document
130 detected.
131 """
132 # Stub out Deckhand render logic.
133 mock_layering.DocumentLayering.return_value.render.return_value = []
134
135 exclude_lint = self._exclude_all(except_code='P004')
136 warn_lint = ['P004']
137 config.set_site_repo(self.site_yaml_path)
138
139 documents = {
140 mock.sentinel.site: [{
141 # Create 2 duplicate DataSchema documents.
142 "schema": "deckhand/DataSchema/v1",
143 "metadata": {
144 "name": mock.sentinel.document_name
145 },
146 "data": {}
147 }] * 2
148 }
149
150 with mock.patch(
151 'pegleg.engine.util.definition.documents_for_each_site',
152 autospec=True,
153 return_value=documents):
154 result = lint.full(
155 False, exclude_lint=exclude_lint, warn_lint=warn_lint)
156 assert len(result) == 1
157 assert result[0].startswith(warn_lint[0])
158
159 @mock.patch('pegleg.engine.util.deckhand.layering', autospec=True)
160 def test_lint_warns_P005(self, mock_layering):
161 """Test warn flag for P005 - Deckhand rendering exception."""
162 # Make Deckhand render expected exception to trigger error code.
163 mock_layering.DocumentLayering.return_value.render.side_effect = (
164 dh_errors.DeckhandException)
165
166 exclude_lint = self._exclude_all(except_code='P005')
167 warn_lint = ['P005']
168 config.set_site_repo(self.site_yaml_path)
169
170 documents = {
171 mock.sentinel.site: [{
172 "schema": "deckhand/DataSchema/v1",
173 "metadata": {
174 "name": mock.sentinel.document_name
175 },
176 "data": {}
177 }]
178 }
179
180 with mock.patch(
181 'pegleg.engine.util.definition.documents_for_each_site',
182 autospec=True,
183 return_value=documents):
184 result = lint.full(
185 False, exclude_lint=exclude_lint, warn_lint=warn_lint)
186 assert len(result) == 1
187 assert result[0].startswith(warn_lint[0])
188
189 def test_lint_warns_P006(self, tmpdir):
190 """Test warn flag for P006 - YAML file missing document header."""
191
192 exclude_lint = self._exclude_all(except_code='P006')
193 warn_lint = ['P006']
194 config.set_site_repo(self.site_yaml_path)
195
196 p = tmpdir.mkdir(self.__class__.__name__).join("test.yaml")
197 p.write("foo: bar")
198
199 with mock.patch(
200 'pegleg.engine.util.files.all',
201 autospec=True,
202 return_value=[p.strpath]):
203 result = lint.full(
204 False, exclude_lint=exclude_lint, warn_lint=warn_lint)
205 assert len(result) == 1
206 assert result[0].startswith(warn_lint[0])
207
208 def test_lint_warns_P007(self, tmpdir):
209 """Test warn flag for P007 - YAML file is not valid YAML."""
210
211 exclude_lint = self._exclude_all(except_code='P007')
212 warn_lint = ['P007']
213 config.set_site_repo(self.site_yaml_path)
214
215 p = tmpdir.mkdir(self.__class__.__name__).join("test.yaml")
216 # Invalid YAML - will trigger error.
217 p.write("---\nfoo: bar: baz")
218
219 with mock.patch(
220 'pegleg.engine.util.files.all',
221 autospec=True,
222 return_value=[p.strpath]):
223 result = lint.full(
224 False, exclude_lint=exclude_lint, warn_lint=warn_lint)
225 assert len(result) == 1
226 assert result[0].startswith(warn_lint[0])
227
228 # TODO(felipemonteiro): Add tests for P008, P009.
92 229
93 230
94class TestSelectableLintingHelperFunctions(object): 231class TestSelectableLintingHelperFunctions(object):
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
index 90bcf43..6743868 100644
--- a/tests/unit/test_utils.py
+++ b/tests/unit/test_utils.py
@@ -23,8 +23,8 @@ import uuid
23 23
24_PROXY_SERVERS = { 24_PROXY_SERVERS = {
25 'http': 25 'http':
26 os.getenv('HTTP_PROXY', 26 os.getenv('HTTP_PROXY', os.getenv('http_proxy',
27 os.getenv('http_proxy', 'http://proxy.example.com')), 27 'http://proxy.example.com')),
28 'https': 28 'https':
29 os.getenv('HTTPS_PROXY', 29 os.getenv('HTTPS_PROXY',
30 os.getenv('https_proxy', 'https://proxy.example.com')) 30 os.getenv('https_proxy', 'https://proxy.example.com'))