Merge branch 'release-0.0.1.dev37'

CHANGELOG.rst

@@ -1,6 +1,14 @@
 Changelog
 =========
 
+0.0.1.dev37 (2020-06-01)
+------------------------
+
+Changed
+~~~~~~~
+
+* Make keyfile optional
+
 0.0.1.dev36 (2020-05-08)
 ------------------------
 

src/gordon_gcp/__init__.py

@@ -15,7 +15,7 @@
 # limitations under the License.
 
 __author__ = 'Lynn Root'
-__version__ = '0.0.1.dev36'
+__version__ = '0.0.1.dev37'
 __license__ = 'Apache 2.0'
 __email__ = 'lynn@spotify.com'
 __description__ = 'GCP Plugins for Gordon and Gordon Janitor'

src/gordon_gcp/plugins/janitor/authority.py

@@ -89,12 +89,6 @@ def _get_gce_client(self, keyfile_path, scopes):
                              blacklisted_metadata=metadata_blacklist)
 
     def _validate_config(self):
-        if not self.config.get('keyfile'):
-            msg = ('The path to a Service Account JSON keyfile is required to '
-                   'authenticate to Google Compute Engine and Cloud '
-                   'Resource Manager.')
-            logging.error(msg)
-            raise exceptions.GCPConfigError(msg)
         if not self.config.get('dns_zone'):
             msg = ('The absolute DNS zone, i.e. "example.com.", is required to '
                    'identify to which zone generated records should belong.')
@@ -103,7 +97,7 @@ def _validate_config(self):
 
     def build_authority(self):
         self._validate_config()
-        keyfile_path = self.config['keyfile']
+        keyfile_path = self.config.get('keyfile')
         scopes = self.config.get('scopes')
         self.session = aiohttp.ClientSession()
         crm_client = self._get_crm_client(keyfile_path, scopes)

src/gordon_gcp/plugins/janitor/gpubsub_publisher.py

@@ -90,14 +90,7 @@ def __init__(self, config, metrics, changes_channel, **kwargs):
         self.kwargs = kwargs
 
     def _validate_config(self):
-        # req keys: keyfile, project, topic
-        # TODO (lynn): keyfile won't be required once we support other
-        #              auth methods
-        if not self.config.get('keyfile'):
-            msg = ('The path to a Service Account JSON keyfile is required to '
-                   'authenticate for Google Cloud Pub/Sub.')
-            logging.error(msg)
-            raise exceptions.GCPConfigError(msg)
+        # req keys: project, topic
         if not self.config.get('project'):
             msg = 'The GCP project where Cloud Pub/Sub is located is required.'
             logging.error(msg)
@@ -125,7 +118,8 @@ def _init_auth(self):
             # uses it but without it aiohttp will complain about an unclosed
             # client session that would otherwise be made by default
             auth_client = auth.GAuthClient(
-                keyfile=self.config['keyfile'], scopes=scopes, session='noop')
+                keyfile=self.config.get('keyfile'),
+                scopes=scopes, session='noop')
         return auth_client
 
     def _init_client(self, auth_client):

src/gordon_gcp/plugins/janitor/reconciler.py

@@ -128,22 +128,16 @@ def __init__(self, config, metrics, rrset_channel, changes_channel,
         self.kwargs = kwargs
 
     def _validate_config(self):
-        # req keys: keyfile, project, topic
-        # TODO (lynn): keyfile won't be required once we support other
-        #              auth methods
-        if not self.config.get('keyfile'):
-            msg = ('The path to a Service Account JSON keyfile is required to '
-                   'authenticate for Google Cloud Pub/Sub.')
-            logging.error(msg)
-            raise exceptions.GCPConfigError(msg)
+        # req keys: project
         if not self.config.get('project'):
             msg = 'The GCP project where Cloud DNS is located is required.'
             logging.error(msg)
             raise exceptions.GCPConfigError(msg)
 
     def _init_auth(self):
         return auth.GAuthClient(
-            keyfile=self.config['keyfile'], scopes=self.config.get('scopes'))
+            keyfile=self.config.get('keyfile'),
+            scopes=self.config.get('scopes'))
 
     def _init_client(self, auth_client):
         kwargs = {

src/gordon_gcp/plugins/service/enricher.py

@@ -61,13 +61,6 @@ def __init__(self, config, metrics, **kwargs):
         self.http_client = self._init_http_client()
         self.dns_client = self._init_dns_client()
 
-    def _validate_keyfile(self):
-        msg = []
-        if not self.config.get('keyfile'):
-            msg.append('The path to a Service Account JSON keyfile is required '
-                       'to authenticate to the GCE API.')
-        return msg
-
     def _validate_dns_zone(self):
         msg = []
         if not self.config.get('dns_zone'):
@@ -97,7 +90,6 @@ def _call_validators(self):
             list(str): Error messages from the validators.
         """
         msg = []
-        msg.extend(self._validate_keyfile())
         msg.extend(self._validate_dns_zone())
         msg.extend(self._validate_retries())
         msg.extend(self._validate_project())
@@ -114,7 +106,7 @@ def _validate_config(self):
 
     def _init_auth(self):
         scopes = self.config.get('scopes')
-        return auth.GAuthClient(keyfile=self.config['keyfile'],
+        return auth.GAuthClient(keyfile=self.config.get('keyfile'),
                                 scopes=scopes)
 
     def _init_http_client(self):

src/gordon_gcp/plugins/service/event_consumer.py

@@ -121,14 +121,7 @@ def __init__(self, config, success_channel, error_channel, metrics,
 
     def _validate_config(self):
         errors = []
-        # req keys: keyfile, project, topic, subscription
-        # TODO (lynn): keyfile won't be required once we support other
-        #              auth methods
-        if not self.config.get('keyfile'):
-            msg = ('The path to a Service Account JSON keyfile is required to '
-                   'authenticate for Google Cloud Pub/Sub.')
-            errors.append(msg)
-
+        # req keys: project, topic, subscription
         if not self.config.get('project'):
             msg = ('The GCP project where Cloud Pub/Sub is located is '
                    'required.')
@@ -180,7 +173,8 @@ def _init_auth(self):
             # uses it but without it aiohttp will complain about an unclosed
             # client session that would otherwise be made by default
             auth_client = auth.GAuthClient(
-                keyfile=self.config['keyfile'], scopes=scopes, session='noop')
+                keyfile=self.config.get('keyfile'),
+                scopes=scopes, session='noop')
         return auth_client
 
     def _init_subscriber_client(self, auth_client):

src/gordon_gcp/plugins/service/gdns_publisher.py

@@ -57,21 +57,12 @@ def __init__(self, config, metrics, **kwargs):
         self.metrics = metrics
         self.kwargs = kwargs
         self.validate_config_funcs = [
-            self._validate_keyfile,
             self._validate_project,
             self._validate_dns_zone,
             self._validate_publish_timeout,
             self._validate_default_ttl
         ]
 
-    def _validate_keyfile(self, errors):
-        # TODO (lynn): keyfile won't be required once we support other
-        #              auth methods
-        if not self.config.get('keyfile'):
-            msg = ('The path to a Service Account JSON keyfile is required to '
-                   'authenticate for Google Cloud DNS.')
-            errors.append(msg)
-
     def _validate_project(self, errors):
         if not self.config.get('project'):
             msg = 'The GCP project where Cloud DNS is located is required.'
@@ -126,7 +117,7 @@ def _validate_config(self):
     def _init_auth_client(self):
         scopes = self.config.get('scopes')
         return auth.GAuthClient(
-            keyfile=self.config['keyfile'], scopes=scopes)
+            keyfile=self.config.get('keyfile'), scopes=scopes)
 
     def _init_dns_client(self):
         auth_client = self._init_auth_client()

tests/unit/clients/test_auth.py

@@ -270,7 +270,6 @@ async def test_refresh_token_with_app_default_cred(client_with_app_default_cred,
                     payload=payload_resp_refresh_token)
         await client_with_app_default_cred.refresh_token()
     assert token == client_with_app_default_cred.token
-    assert 2 == len(caplog.records)
 
 
 @pytest.mark.asyncio

tests/unit/plugins/janitor/test_init.py

@@ -52,7 +52,6 @@ def test_get_gpubsub_publisher(local, timeout, exp_timeout, topic, config,
 
 
 @pytest.mark.parametrize('config_key,exp_msg', [
-    ('keyfile', 'The path to a Service Account JSON keyfile is required '),
     ('project', 'The GCP project where Cloud Pub/Sub is located is required.'),
     ('topic', ('A topic for the client to publish to in Cloud Pub/Sub is '
                'required.')),
@@ -130,7 +129,6 @@ def test_get_reconciler(timeout, exp_timeout, config, auth_client, monkeypatch,
 
 
 @pytest.mark.parametrize('key,error_msg', [
-    ('keyfile', 'The path to a Service Account JSON keyfile is required '),
     ('project', 'The GCP project where Cloud DNS is located is required.')
 ])
 def test_get_reconciler_config_raises(key, error_msg, config, auth_client,
@@ -163,7 +161,6 @@ async def test_get_authority(authority_config, auth_client, metrics):
 
 
 @pytest.mark.parametrize('config_key,error_msg', [
-    ('keyfile', 'The path to a Service Account JSON keyfile is required '),
     ('dns_zone', 'The absolute DNS zone, i.e. "example.com.", is required ')])
 def test_get_authority_config_raises(caplog, config_key, error_msg,
                                      authority_config, auth_client, metrics):

tests/unit/plugins/service/test_init.py

@@ -111,7 +111,6 @@ def test_get_event_consumer(local, provide_loop, topic, sub, consumer_config,
 
 
 @pytest.mark.parametrize('config_key,exp_msg',  [
-    ('keyfile', 'The path to a Service Account JSON keyfile is required '),
     ('project', 'The GCP project where Cloud Pub/Sub is located is required.'),
     ('topic', ('A topic for the client to subscribe to in Cloud Pub/Sub is '
                'required.')),
@@ -256,8 +255,6 @@ def test_get_enricher(mocker, enricher_config, auth_client, conf_retries,
 
 
 @pytest.mark.parametrize('config_key,exc_msg', [
-    ('keyfile', 'The path to a Service Account JSON keyfile is required to '
-                'authenticate to the GCE API.'),
     ('dns_zone', 'A dns zone is required to build correct A records.'),
     ('project', 'The GCP project that contains the Google Cloud DNS managed '
                 'zone is required to correctly delete A records for deleted '
@@ -330,16 +327,10 @@ def test_get_gdns_publisher(conf_key, conf_value, expected, mocker,
 
 
 @pytest.mark.parametrize('conf_keys,exp_msg_snip', (
-    (('keyfile',), ('The path to a Service Account JSON keyfile is required '
-                    'to authenticate for Google Cloud DNS.')),
     (('project',), 'The GCP project where Cloud DNS is located is required.'),
     (('dns_zone',), 'A dns zone is required to build correct A records.'),
     (('default_ttl',), ('A default TTL in seconds must be set for publishing '
                         'records to Google Cloud DNS.')),
-    (('keyfile', 'project'), ('The path to a Service Account JSON keyfile is '
-                              'required to authenticate for Google Cloud DNS.\n'
-                              'The GCP project where Cloud DNS is located is '
-                              'required.\n'))
 ))
 def test_get_gdns_publisher_raises(conf_keys, exp_msg_snip,
                                    publisher_config, mocker, auth_client,