diff --git a/google/cloud/forseti/scanner/audit/ke_rules_engine.py b/google/cloud/forseti/scanner/audit/ke_rules_engine.py index 14f4037c7a..2feea82623 100644 --- a/google/cloud/forseti/scanner/audit/ke_rules_engine.py +++ b/google/cloud/forseti/scanner/audit/ke_rules_engine.py @@ -314,7 +314,19 @@ def find_violations(self, ke_cluster): """ violations = [] - actual = self.rule_jmespath.search(ke_cluster.as_dict) + try: + actual = self.rule_jmespath.search(ke_cluster.as_dict) + except jmespath.exceptions.JMESPathError as e: + LOGGER.warning( + 'JMESPath error processing KE cluster %s: %s', + ke_cluster.id, + e + ) + + # The resource data violated some assumption the user made + # about it. So we cannot know if any violations occurred. + return violations + LOGGER.debug('actual jmespath result: %s', actual) if self.rule_mode == 'whitelist': diff --git a/tests/scanner/scanners/data/ke_scanner_test_data.yaml b/tests/scanner/scanners/data/ke_scanner_test_data.yaml index 4b3251ca35..eb04980b19 100644 --- a/tests/scanner/scanners/data/ke_scanner_test_data.yaml +++ b/tests/scanner/scanners/data/ke_scanner_test_data.yaml @@ -111,3 +111,34 @@ rules: mode: whitelist values: - True + + # If a cluster has no node pools, the nodePools field is not + # returned by the GKE API. Thus scanner doesn't have enough + # information to get this test rule correct. But still, scanner + # shouldn't raise an exception and fail. So this is just ensuring + # the fix to #2665 fixes the crash. See also the next rule. + + - name: missing nodePool, should not generate violation + resource: + - type: organization + resource_ids: + - '*' + key: 'length(nodePools) > `0`' + mode: whitelist + values: + - true + + # This rule uses OR to handle clusters with no node pools. Not only + # should this rule not produce an exception, it should actually + # detect the test cluster data with no node pools. See also the + # previous rule. + + - name: missing nodePool, should generate violation + resource: + - type: organization + resource_ids: + - '*' + key: 'length(nodePools || `[]`) > `0`' + mode: whitelist + values: + - true diff --git a/tests/scanner/scanners/ke_scanner_test.py b/tests/scanner/scanners/ke_scanner_test.py index 7698e17fc2..dccf8963db 100644 --- a/tests/scanner/scanners/ke_scanner_test.py +++ b/tests/scanner/scanners/ke_scanner_test.py @@ -62,8 +62,27 @@ FAKE_CLUSTER_ID = size_t_hash('fake-cluster.com') +NO_NODE_POOLS = ''' +{ + "name": "fake-cluster-no-node-pools", + "addonsConfig": { + "httpLoadBalancing": {}, + "kubernetesDashboard": { + "disabled": true + }, + "istioConfig": { + "auth": "AUTH_MUTUAL_TLS" + } + }, + "selfLink": "fake-cluster-no-node-pools.com" +} +''' + +NO_NODE_POOLS_ID = size_t_hash('fake-cluster-no-node-pools.com') + FAKE_CLUSTERS = { FAKE_CLUSTER_ID: FAKE_CLUSTER, + NO_NODE_POOLS_ID: NO_NODE_POOLS, } @@ -92,20 +111,21 @@ def setUpClass(cls): project = data_access.add_resource(session, 'project/fake-project', organization) - ke_cluster = data_access.add_resource( - session, - 'kubernetes_cluster/{}'.format(FAKE_CLUSTER_ID), - project, - ) + for cluster_id, cluster in FAKE_CLUSTERS.items(): + ke_cluster = data_access.add_resource( + session, + 'kubernetes_cluster/{}'.format(cluster_id), + project, + ) - ke_cluster.data = FAKE_CLUSTER + ke_cluster.data = cluster - sc = data_access.add_resource( - session, - 'kubernetes_service_config/{}'.format(FAKE_CLUSTER_ID), - ke_cluster, - ) - sc.data = SERVER_CONFIG + sc = data_access.add_resource( + session, + 'kubernetes_service_config/{}'.format(cluster_id), + ke_cluster, + ) + sc.data = SERVER_CONFIG session.commit() @@ -116,12 +136,88 @@ def setUp(self): '', unittest_utils.get_datafile_path( __file__, 'ke_scanner_test_data.yaml')) + @mock.patch( + 'google.cloud.forseti.scanner.audit.ke_rules_engine.LOGGER', + autospsec=True, + ) @mock.patch.object( ke_scanner.KeScanner, '_output_results_to_db', autospec=True) - def test_run_scanner(self, mock_output_results): + def test_run_scanner(self, mock_output_results, mock_logger): self.scanner.run() expected_violations = [ + {'rule_name': 'explicit whitelist, pass', + 'resource_name': 'fake-cluster-no-node-pools', + 'resource_data': '{"addonsConfig": {"httpLoadBalancing": {}, "istioConfig": {"auth": "AUTH_MUTUAL_TLS"}, "kubernetesDashboard": {"disabled": true}}, "name": "fake-cluster-no-node-pools", "selfLink": "fake-cluster-no-node-pools.com"}', + 'full_name': 'organization/12345/project/fake-project/kubernetes_cluster/{}/'.format(NO_NODE_POOLS_ID), + 'resource_id': NO_NODE_POOLS_ID, + 'rule_index': 0, + 'violation_type': 'KE_VIOLATION', + 'violation_data': {'violation_reason': "name has value fake-cluster-no-node-pools, which is not in the whitelist (['fake-cluster'])", 'cluster_name': 'fake-cluster-no-node-pools', 'project_id': 'fake-project', 'full_name': 'organization/12345/project/fake-project/kubernetes_cluster/{}/'.format(NO_NODE_POOLS_ID)}, + 'resource_type': 'kubernetes_cluster'}, + + {'rule_name': 'explicit whitelist, fail', + 'resource_name': u'fake-cluster-no-node-pools', + 'resource_data': '{"addonsConfig": {"httpLoadBalancing": {}, "istioConfig": {"auth": "AUTH_MUTUAL_TLS"}, "kubernetesDashboard": {"disabled": true}}, "name": "fake-cluster-no-node-pools", "selfLink": "fake-cluster-no-node-pools.com"}', + 'full_name': 'organization/12345/project/fake-project/kubernetes_cluster/{}/'.format(NO_NODE_POOLS_ID), + 'resource_id': NO_NODE_POOLS_ID, + 'rule_index': 1, + 'violation_type': 'KE_VIOLATION', + 'violation_data': {'violation_reason': u"name has value fake-cluster-no-node-pools, which is not in the whitelist (['real-cluster'])", 'cluster_name': u'fake-cluster-no-node-pools', 'project_id': 'fake-project', 'full_name': 'organization/12345/project/fake-project/kubernetes_cluster/{}/'.format(NO_NODE_POOLS_ID)}, + 'resource_type': 'kubernetes_cluster'}, + + {'rule_name': 'multiple values, pass', + 'resource_name': u'fake-cluster-no-node-pools', + 'resource_data': '{"addonsConfig": {"httpLoadBalancing": {}, "istioConfig": {"auth": "AUTH_MUTUAL_TLS"}, "kubernetesDashboard": {"disabled": true}}, "name": "fake-cluster-no-node-pools", "selfLink": "fake-cluster-no-node-pools.com"}', + 'full_name': 'organization/12345/project/fake-project/kubernetes_cluster/{}/'.format(NO_NODE_POOLS_ID), + 'resource_id': NO_NODE_POOLS_ID, + 'rule_index': 4, + 'violation_type': 'KE_VIOLATION', + 'violation_data': {'violation_reason': u"name has value fake-cluster-no-node-pools, which is not in the whitelist (['real-cluster', 'fake-cluster'])", 'cluster_name': u'fake-cluster-no-node-pools', 'project_id': 'fake-project', 'full_name': 'organization/12345/project/fake-project/kubernetes_cluster/{}/'.format(NO_NODE_POOLS_ID)}, + 'resource_type': 'kubernetes_cluster'}, + + {'rule_name': 'multiple values, fail', + 'resource_name': u'fake-cluster-no-node-pools', + 'resource_data': '{"addonsConfig": {"httpLoadBalancing": {}, "istioConfig": {"auth": "AUTH_MUTUAL_TLS"}, "kubernetesDashboard": {"disabled": true}}, "name": "fake-cluster-no-node-pools", "selfLink": "fake-cluster-no-node-pools.com"}', + 'full_name': 'organization/12345/project/fake-project/kubernetes_cluster/{}/'.format(NO_NODE_POOLS_ID), + 'resource_id': NO_NODE_POOLS_ID, + 'rule_index': 5, + 'violation_type': 'KE_VIOLATION', + 'violation_data': {'violation_reason': u"name has value fake-cluster-no-node-pools, which is not in the whitelist (['real-cluster', 'imaginary-cluster'])", 'cluster_name': u'fake-cluster-no-node-pools', 'project_id': 'fake-project', 'full_name': 'organization/12345/project/fake-project/kubernetes_cluster/{}/'.format(NO_NODE_POOLS_ID)}, + 'resource_type': 'kubernetes_cluster'}, + + {'rule_name': 'use projection to look for a list, pass', + 'resource_name': u'fake-cluster-no-node-pools', + 'resource_data': '{"addonsConfig": {"httpLoadBalancing": {}, "istioConfig": {"auth": "AUTH_MUTUAL_TLS"}, "kubernetesDashboard": {"disabled": true}}, "name": "fake-cluster-no-node-pools", "selfLink": "fake-cluster-no-node-pools.com"}', + 'full_name': 'organization/12345/project/fake-project/kubernetes_cluster/{}/'.format(NO_NODE_POOLS_ID), + 'resource_id': NO_NODE_POOLS_ID, + 'rule_index': 7, 'violation_type': 'KE_VIOLATION', + 'violation_data': {'violation_reason': "nodePools[*].config.imageType has value None, which is not in the whitelist ([['COS', 'COS']])", 'cluster_name': u'fake-cluster-no-node-pools', 'project_id': 'fake-project', 'full_name': 'organization/12345/project/fake-project/kubernetes_cluster/{}/'.format(NO_NODE_POOLS_ID)}, + 'resource_type': 'kubernetes_cluster'}, + + {'rule_name': 'use projection, look for a list, fail', + 'resource_name': u'fake-cluster-no-node-pools', + 'resource_data': '{"addonsConfig": {"httpLoadBalancing": {}, "istioConfig": {"auth": "AUTH_MUTUAL_TLS"}, "kubernetesDashboard": {"disabled": true}}, "name": "fake-cluster-no-node-pools", "selfLink": "fake-cluster-no-node-pools.com"}', + 'full_name': 'organization/12345/project/fake-project/kubernetes_cluster/{}/'.format(NO_NODE_POOLS_ID), + 'resource_id': NO_NODE_POOLS_ID, + 'rule_index': 8, + 'violation_type': 'KE_VIOLATION', + 'violation_data': {'violation_reason': "nodePools[*].config.imageType has value None, which is not in the whitelist ([['COS'], ['Ubuntu', 'COS']])", 'cluster_name': u'fake-cluster-no-node-pools', 'project_id': 'fake-project', 'full_name': 'organization/12345/project/fake-project/kubernetes_cluster/{}/'.format(NO_NODE_POOLS_ID)}, + 'resource_type': 'kubernetes_cluster'}, + + {'rule_name': 'missing nodePool, should generate violation', + 'resource_name': 'fake-cluster-no-node-pools', + 'resource_data': '{"addonsConfig": {"httpLoadBalancing": {}, "istioConfig": {"auth": "AUTH_MUTUAL_TLS"}, "kubernetesDashboard": {"disabled": true}}, "name": "fake-cluster-no-node-pools", "selfLink": "fake-cluster-no-node-pools.com"}', + 'full_name': 'organization/12345/project/fake-project/kubernetes_cluster/{}/'.format(NO_NODE_POOLS_ID), + 'resource_id': NO_NODE_POOLS_ID, + 'rule_index': 12, + 'violation_type': 'KE_VIOLATION', + 'violation_data': {'violation_reason': 'length(nodePools || `[]`) > `0` has value False, which is not in the whitelist ([True])', + 'cluster_name': 'fake-cluster-no-node-pools', + 'project_id': 'fake-project', + 'full_name': 'organization/12345/project/fake-project/kubernetes_cluster/{}/'.format(NO_NODE_POOLS_ID)}, + 'resource_type': 'kubernetes_cluster'}, + {'rule_name': 'explicit whitelist, fail', 'resource_name': u'fake-cluster', 'full_name': u'organization/12345/project/fake-project/kubernetes_cluster/{}/'.format(FAKE_CLUSTER_ID), @@ -153,6 +249,14 @@ def test_run_scanner(self, mock_output_results): mock_output_results.assert_called_once_with(mock.ANY, expected_violations) + # check that the "missing nodePool, should not generate + # violation" rule test case did in fact log + self.assertTrue(mock_logger.warning.called) + self.assertTrue( + 'JMESPath error processing KE cluster %s:' in mock_logger.warning.call_args[0][0], + ) + self.assertTrue(NO_NODE_POOLS_ID in mock_logger.warning.call_args[0][1]) + if __name__ == '__main__': unittest.main()