Prefer signature version from service models over endpoints.json

[ashovlin]

Background

An upcoming service is re-using an existing endpoint prefix that is still present in endpoints.json with an explicit signatureVersions.

The new service is specifying a different signatureVersion in its service model. However botocore is still preferring the version from endpoints.json

#2816 in 2022 last touched this area. However this logic:

botocore/botocore/client.py

Lines 836 to 843 in 6f783fe

	if (
	self.service_signature_version is not None
	and self.service_signature_version
	not in _LEGACY_SIGNATURE_VERSIONS
	):
	# Prefer the service model as most specific
	# source of truth for new signature versions.
	potential_versions = [self.service_signature_version]

only prefers the service model over endpoints.json if the service model's signature version is not in the this list:

botocore/botocore/client.py

Lines 69 to 78 in 6f783fe

	_LEGACY_SIGNATURE_VERSIONS = frozenset(
	(
	'v2',
	'v3',
	'v3https',
	'v4',
	's3',
	's3v4',
	)
	)

My interpretation is that this allowed new services to model new signature versions such as bearer, but preserved our original behavior of using endpoints.json as the source of truth.

Approach

Now, I removed that _LEGACY_SIGNATURE_VERSIONS set. We'll prefer the service model it its set, but still fall back to endpoints.json if not.

This required some custom handling for S3, S3Control, and Import/Export to preserve their current resolved values.

For Reviewers: Long term, we likely want to remove endpoints.json entirely. I didn't do so yet, as that resolved is plumbed around pretty heavily, and we do have unit tests that are guarding the current code paths. But let me know if you'd rather do so now.

Testing

1. Added a unit test exercising the scenario that the upcoming service is hitting.
2. Ran the following both before and after this change, confirmed that there was not a diff in the output:

import json
import botocore.session

session = botocore.session.get_session()
signature_versions = {}
for service_name in session.get_available_services():
    client = session.create_client(service_name)
    signature_versions[service_name] = client.meta.config.signature_version

print(json.dumps(signature_versions, indent=2))

[ashovlin]

Unsure if we want this, as it should be transparent. #2816 didn't have a changelog entry.

[nateprewitt]

This is probably a nuance we don't need to highlight. These aren't public interfaces and as long as the change is a no-op for the end user, it's kind of moot.

[ashovlin]

Removed via https://github.com/boto/botocore/pull/3465/commits/0da938819815bf35605d85519752de3c0a2732ef/

[nateprewitt]

This is probably a nuance we don't need to highlight. These aren't public interfaces and as long as the change is a no-op for the end user, it's kind of moot.

[nateprewitt]

Do we have any other options for this? S3 has it's own special cases throughout the code but adding more random service conditionals is ideally avoided. I'd maybe suggest we add a handler to set this explicitly for the service instead.

[ashovlin]

One note on the approach in b2da232:

client.meta.config.signature_version for importexport will now return v2 (instead of v4) since this doesn't execute the choose-signer event.

I removed that assertion from the functional test, but left the assertion that the request ultimately has the V4 header.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.04%. Comparing base (6300f6b) to head (1724736).
Report is 254 commits behind head on develop.

Files with missing lines	Patch %	Lines
botocore/client.py	95.00%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3465      +/-   ##
===========================================
+ Coverage    92.94%   93.04%   +0.10%     
===========================================
  Files           66       67       +1     
  Lines        14956    15132     +176     
===========================================
+ Hits         13901    14080     +179     
+ Misses        1055     1052       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[nateprewitt]

Comment on a nit, but otherwise LGTM.