feat: deprecate master key providers #242
Conversation
README.rst
Outdated
| An example of a master key provider is `AWS KMS`_. | ||
| The default CMM gets the encryption or decryption materials from | ||
| the keyring or master key provider that you specify. | ||
| This might involve a call to a cryptographic service, such as AWS Key Management Service (AWS KMS). |
There was a problem hiding this comment.
Not sure you need to bring this up here, maybe in the keyring section?
There was a problem hiding this comment.
This and the descriptions for keyrings and data keys are just copied from the developer guide concepts docs. Thinking this over, to avoid needing to try to keep these in sync, I'm just going to replace these with a single sentence intro and a link to the appropriate section in the dev guide.
README.rst
Outdated
|
|
||
| To encrypt data in this client, a ``MasterKeyProvider`` object must contain at least one ``MasterKey`` object. | ||
| You can specify a CMM and master key provider or keyring, but it's not required. |
There was a problem hiding this comment.
This sentence is a little difficult to parse, I can't tell what is not required. Maybe "You can specify either a CMM, a master key provider, or a keyring"
README.rst
Outdated
| An example of a master key is a `KMS customer master key (CMK)`_. | ||
| A keyring generates, encrypts, and decrypts data keys. | ||
| Each keyring is typically associated with a wrapping key or a service that provides and protects wrapping keys. | ||
| You can use the keyrings that the AWS Encryption SDK provides or write your own compatible custom keyrings. |
There was a problem hiding this comment.
Not sure what "compatible" is bringing to this sentence
| @@ -78,6 +83,16 @@ def __new__(cls, **kwargs): | |||
| """Set key index and member set for all new instances here | |||
| to avoid requiring child classes to call super init. | |||
| """ | |||
| # DeprecationWarning are ignored by default, | |||
| # but because we do not have a plan to remove master key providers, | |||
There was a problem hiding this comment.
I don't like the sound of "we do not have a plan". Maybe just remove these lines and say "Once we decide that we are one X or Y version away from removing master key providers, we should upgrade the DeprecationWarning to a UserWarning so the warning is not ignored by default."
CHANGELOG.rst
Outdated
|
|
||
| * Deprecate master key providers in favor of keyrings. | ||
|
|
||
| * We do not have a planned version for when we will remove master key providers. |
There was a problem hiding this comment.
This is a little pessimistic, how about something like "Master key providers continue to be supported. Prior to their removal, we will communicate this as defined in our versioning policy."
Co-Authored-By: Wesley Rosenblum <55108558+WesleyRosenblum@users.noreply.github.com>
Issue #, if available:
resolves: #238
Description of changes:
I went with
DeprecationWarningeven though it is ignored by default because we do not have any plans for when we will actually remove master key providers. When that changes, we should up that to aUserWarningand communicate the upcoming removal as defined in our versioning policy.https://docs.python.org/3/library/warnings.html#warning-categories
In addition to adding deprecation notices, this also removes any use of master key providers from inline examples. For the readme I decided to just remove all of the inline examples because we have a much more comprehensive set of examples in the examples directory now. The new concepts descriptions in the readme are taken from developer guide's concepts section.
https://docs.aws.amazon.com/encryption-sdk/latest/developer-guide/concepts.html
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: