feat: change KMS discovery keyring configuration path (#228)

* feat: change KMS discovery keyring configuration path

* fix: clarify error message when insufficient configuration is provided to KmsKeyring

Author: mattsb42-aws

src/aws_encryption_sdk/keyrings/aws_kms/__init__.py

@@ -55,10 +55,13 @@ class KmsKeyring(Keyring):
     but for the keyring to attempt to use them on decrypt
     you MUST specify the CMK ARN.
 
-    If you specify neither ``generator_key_id`` nor ``child_key_ids``
-    then the keyring will operate in `discovery mode`_,
+    If you specify ``is_discovery=True`` the keyring will be a KMS discovery keyring,
     doing nothing on encrypt and attempting to decrypt any AWS KMS-encrypted data key on decrypt.
 
+    .. notice::
+
+        You must either set ``is_discovery=True`` or provide key IDs.
+
     You can use the :class:`ClientSupplier` to customize behavior further,
     such as to provide different credentials for different regions
     or to restrict which regions are allowed.
@@ -75,12 +78,14 @@ class KmsKeyring(Keyring):
     .. versionadded:: 1.5.0
 
     :param ClientSupplier client_supplier: Client supplier that provides AWS KMS clients (optional)
+    :param bool is_discovery: Should this be a discovery keyring (optional)
     :param str generator_key_id: Key ID of AWS KMS CMK to use when generating data keys (optional)
     :param List[str] child_key_ids: Key IDs that will be used to encrypt and decrypt data keys (optional)
     :param List[str] grant_tokens: AWS KMS grant tokens to include in requests (optional)
     """
 
     _client_supplier = attr.ib(default=attr.Factory(DefaultClientSupplier), validator=is_callable())
+    _is_discovery = attr.ib(default=False, validator=instance_of(bool))
     _generator_key_id = attr.ib(default=None, validator=optional(instance_of(six.string_types)))
     _child_key_ids = attr.ib(
         default=attr.Factory(tuple),
@@ -93,6 +98,22 @@ class KmsKeyring(Keyring):
 
     def __attrs_post_init__(self):
         """Configure internal keyring."""
+        key_ids_provided = self._generator_key_id is not None or self._child_key_ids
+        both = key_ids_provided and self._is_discovery
+        neither = not key_ids_provided and not self._is_discovery
+
+        if both:
+            raise TypeError("is_discovery cannot be True if key IDs are provided")
+
+        if neither:
+            raise TypeError("is_discovery cannot be False if no key IDs are provided")
+
+        if self._is_discovery:
+            self._inner_keyring = _AwsKmsDiscoveryKeyring(
+                client_supplier=self._client_supplier, grant_tokens=self._grant_tokens
+            )
+            return
+
         if self._generator_key_id is None:
             generator_keyring = None
         else:
@@ -107,14 +128,7 @@ def __attrs_post_init__(self):
             for key_id in self._child_key_ids
         ]
 
-        self._is_discovery = generator_keyring is None and not child_keyrings
-
-        if self._is_discovery:
-            self._inner_keyring = _AwsKmsDiscoveryKeyring(
-                client_supplier=self._client_supplier, grant_tokens=self._grant_tokens
-            )
-        else:
-            self._inner_keyring = MultiKeyring(generator=generator_keyring, children=child_keyrings)
+        self._inner_keyring = MultiKeyring(generator=generator_keyring, children=child_keyrings)
 
     def on_encrypt(self, encryption_materials):
         # type: (EncryptionMaterials) -> EncryptionMaterials

test/unit/keyrings/test_aws_kms.py

@@ -24,6 +24,9 @@
         pytest.param(dict(child_key_ids="some stuff"), id="child_key_ids is a string"),
         pytest.param(dict(grant_tokens=("foo", 5)), id="grant_tokens contains invalid values"),
         pytest.param(dict(grant_tokens="some stuff"), id="grant_tokens is a string"),
+        pytest.param(dict(generator_key_id="foo", is_discovery=True), id="generator and discovery"),
+        pytest.param(dict(child_key_ids=("foo",), is_discovery=True), id="child_key_ids and discovery"),
+        pytest.param(dict(), id="nothing"),
     ),
 )
 def test_kms_keyring_invalid_parameters(kwargs):
@@ -100,7 +103,7 @@ def test_kms_keyring_builds_correct_inner_keyring_discovery():
     grants = ("asdf", "fdas")
     supplier = DefaultClientSupplier()
 
-    test = KmsKeyring(grant_tokens=grants, client_supplier=supplier)
+    test = KmsKeyring(is_discovery=True, grant_tokens=grants, client_supplier=supplier)
 
     # We specified neither a generator nor children, so the inner keyring MUST be a discovery keyring
     assert isinstance(test._inner_keyring, _AwsKmsDiscoveryKeyring)
@@ -110,10 +113,10 @@ def test_kms_keyring_builds_correct_inner_keyring_discovery():
     assert test._inner_keyring._client_supplier is supplier
 
 
-def test_kms_keyring_on_encrypt(mocker):
+def test_kms_keyring_inner_keyring_on_encrypt(mocker):
     mock_keyring = mocker.Mock()
 
-    keyring = KmsKeyring()
+    keyring = KmsKeyring(is_discovery=True)
     keyring._inner_keyring = mock_keyring
 
     test = keyring.on_encrypt(encryption_materials=mocker.sentinel.encryption_materials)
@@ -123,10 +126,10 @@ def test_kms_keyring_on_encrypt(mocker):
     assert test is mock_keyring.on_encrypt.return_value
 
 
-def test_kms_keyring_on_decrypt(mocker):
+def test_kms_keyring_inner_keyring_on_decrypt(mocker):
     mock_keyring = mocker.Mock()
 
-    keyring = KmsKeyring()
+    keyring = KmsKeyring(is_discovery=True)
     keyring._inner_keyring = mock_keyring
 
     test = keyring.on_decrypt(