WIP: Feat/boto3 refactoring#58624
Closed
major0 wants to merge 9 commits into
Closed
Conversation
ghost
requested a review
from
This was referenced Oct 4, 2020
major0
force-pushed
the
feat/boto3_refactoring
branch
6 times, most recently
from
e78c9e5 to
04ad3af
Compare
krionbsd
previously approved these changes
Oct 5, 2020
major0
force-pushed
the
feat/boto3_refactoring
branch
from
04ad3af to
0df46d0
Compare
A number of boto3 modules contain boto checks and imports, but do not actually make any boto calls via the boto libraries.
The previous code looks to be a minor change of the boto `get_connection()` code. In particular, `boto3.session.Session()` generally does not perform any form of authentication or region validation, and it certainly does not raise `boto.exception.NoAuthHandlerFound`. Have rewritten the code to not attempt to use the old boto 2 module.
Add a simple http request to test to see if the saltmaster is running within the AWS infrastructure and assume the instance's region as the default region. If this test fails then fall-back to the hard-coded default of `us-east-1`.
This module did not previously have any unit tests defined.
major0
force-pushed
the
feat/boto3_refactoring
branch
from
2ddfea1 to
a0fcdbd
Compare
Contributor
Author
|
This PR has been split into a number of smaller PRs to make it more approachable: |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Attempt to clearly de-couple AWS boto and boto3 and outline a clear path for removing the now deprecated AWS boto library.
This is a merger of two previous PR's as I was unable to cleanly separate them while still getting them to pass all the pre-commit and unit testing requirements.
Previous Behavior
botoandboto3even if they never usedboto, and in a few cases they attempt to use both at the same time (even though they are generally incompatible).From
boto_iam.py:boto3mod.get_connection()has a number of conditionals that will never fail (copy/pasted from botomod). Further more, these conditionals are in atry/exceptblock which tries to catch abotoexception that is not defined inboto3.salt.utils.version.check_boto_req()helper function is written in such a way that it defaults to assuming all modules need to use boto && boto3 (even though that is not desired).New Behavior
salt.utils.version.check_boto_reqs()and require explicit versions (and disallow specifying versions for boto and boto3 at the same time).boto3mod.get_connection()to work with boto3boto3mod.get_region()to attempt to use the local instance region by default, and if that fails to fall-back onus-east-1.Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
No
Please review Salt's Contributing Guide for best practices.
See GitHub's page on GPG signing for more information about signing commits with GPG.