Fixed KMS master key provider tests when default AWS region is configured by ragona · Pull Request #179 · aws/aws-encryption-sdk-python (original) (raw)
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
ragona merged 3 commits intoaws:masterfromragona:default-region-test-fixes
Aug 2, 2019
Conversation15 Commits3 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
This fixes issue #31, in which users who have configured a default AWS region (either via ~/.aws/config or via environment variable) will experience failing tests for KMSMasterKeyProvider. I tried to keep the tests mostly unchanged, but in some cases I elected to simply skip them since the test didn't provide much value if there was already a default value configured.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General note: reading back through these, the intent of some of these tests are a bit difficult to determine. Feel free to rename any tests if you think a different name would better communicate their intent.
Thanks for the very helpful comments, @mattsb42-aws! I think I've responded to all of your comments with a much-improved way to fix this that doesn't involve skipping any tests. I'm not sure if I followed the general best practices and conventions for creating that fixture though, so definitely looking for feedback on general style.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this, but there's one thing I want to add to it before merging.
I want to make sure that we are testing everything in this module both with a default region set and not set to make sure that we do not regress.
For simplicity, I'm just going to push a commit to your branch, because this gets a bit into dark pytest magics.
…gainst both with and without default region
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming Travis passes, this LGTM.
@ragona before merging, please take a look at the fixture I added and make sure it makes sense to you. If not, we'll expand docs/etc before merging.
mattsb42-aws added a commit that referenced this pull request
Update PR template
Added a check for max_age being greater than 0 (#172)
Added a check for max_age being greater than 0
Fixed flake8 by adding missing pydocstyle dependency
Added the dependency to decrypt_oracle as well
Added test for max_age<=0 ValueError
Updated test for max_age<=0.0 ValueError
Added negative test case
Fixed KMS master key provider tests when default AWS region is configured (#179)
Fixed KMS master key provider tests for users who have their default AWS region configured
created fixture for botocore session with no region set
add auto-used fixture in KMS master key provider unit tests to test against both with and without default region
Wrote example and test for using one kms cmk with an unsigned algorithm
Update one_kms_cmk_unsigned.py
Update examples/src/one_kms_cmk_unsigned.py
Co-Authored-By: Matt Bullock bullocm@amazon.com
isort-check now succeeds
[issue-190] Regional clients modify default botocore session (#193)
[issue-190] Creation of regional clients modifies default botocore session's region
update changelog with changes for 1.4.1 release
bump version to 1.4.1
Updates to handle new pylint requirements (#196)
pylint max-attributes appears to be ratcheted down recently
remove unnecessary comprehensions
whitelist some pylint use-constant-test false-positives
reorganize backwards compatibility test requirements definitions attrs==19.2.0 removed a deprecated feature that aws-encryption-sdk==1.3.3 depended on. This reorganization lets us define specific requirements bounds for old versions of aws-encryption-sdk that will probably continue to be necessary as these old versions age.
remove unnecessary comprehensions
add newlines to the end of all requirements files
help pylint ignore mypy type use
mattsb42-aws added a commit that referenced this pull request
Update PR template
Added a check for max_age being greater than 0 (#172)
Added a check for max_age being greater than 0
Fixed flake8 by adding missing pydocstyle dependency
Added the dependency to decrypt_oracle as well
Added test for max_age<=0 ValueError
Updated test for max_age<=0.0 ValueError
Added negative test case
Testing something, want AppVeyor to run
Quick change
Running AppVeyor
Added example for using multiple keyrings in multiple regions
Undid something quickly
Fixed importerror
Formatting fix
Update tox.ini
Update tox.ini
Made some changes to the multiple_kms_cmk_regions example/test
This is my next interation of the code for the example; however, I am still working on populating the tests correctly, so the CI will fail, but I tested the code with my own KMS CMK ARNs, so I know it will work once the tests are populated (working with Tejeswini on this)
Changed the example to test two CMKs in the same region until Issue #178 is cleared up
Found out how to make a new valid test key, so now there are two valid test keys in different regions for this example
Ran autoformat
Added some docstrings
Formatting will be the death of me
Used correct keys in test
Updated some comments
Fixed KMS master key provider tests when default AWS region is configured (#179)
Fixed KMS master key provider tests for users who have their default AWS region configured
created fixture for botocore session with no region set
add auto-used fixture in KMS master key provider unit tests to test against both with and without default region
Wrote example and test for using one kms cmk with an unsigned algorithm
Update the integration tests
Small changes
Update one_kms_cmk_unsigned.py
Update examples/src/one_kms_cmk_unsigned.py
Co-Authored-By: Matt Bullock bullocm@amazon.com
isort-check now succeeds
chore: move existing examples into "legacy" directory
chore: add automatic test runner for examples
chore: convert existing examples to work with automatic test runner
chore: move examples kwarg building into utils module
chore: convert multi-CMK test runners to new configuration format
fix: fix multi-CMK example logic
chore: convert multi-CMK example to new test runner signature and move into legacy examples
chore: make examples linting always run across both source and tests
fix: linting fixes
docs: add docstring description for legacy examples module
chore: add initial new-format examples
docs: add examples readme
docs: add instructions for writing examples
chore: address PR comments
chore: change examples parameter from aws_kms_cmk_arn to aws_kms_cmk for consistency
docs: clarify integration tests readme
Apply suggestions from code review
Co-Authored-By: June Blender juneb@users.noreply.github.com
- Apply suggestions from code review
Co-Authored-By: June Blender juneb@users.noreply.github.com
docs: change from "one-shot" term to "one-step"
chore: apply changes based on PR comments
docs: reword parameter description
Apply suggestions from code review
Co-Authored-By: June Blender juneb@users.noreply.github.com
chore: rename examples input parameters to move from "child" to "additional" naming
docs: clarify configuration intro
docs: apply examples comments consistently
chore: fix line length
fix: fix typo
Co-authored-by: John Walker jhwal@amazon.com Co-authored-by: Caitlin Tibbetts caitlin@tibbettsfamily.com Co-authored-by: Caitlin Tibbetts tibbetc@amazon.com Co-authored-by: Ryan Ragona ryanragona@gmail.com Co-authored-by: lizroth 30636882+lizroth@users.noreply.github.com Co-authored-by: June Blender juneb@users.noreply.github.com
2 participants
