-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[v2] Add configuration to disable host prefix injection #9268
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just a couple small suggestions. I'm also curious if there are existing unit/functional tests where you can actually set the environment variable or create a config file to test that they resolve correctly E2E.
Edit: Chatted offline about setting env vars/config file. Decided we're okay with current tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we wait to merge until we figure out the plan for botocore? Want to understand where V1 vs. V2 will land.
4ab1cc6
to
e764f66
Compare
We should still wait to merge once we confirm that this is going to be fine for |
I think we can - this should have been done for configured endpoint URLs. I'll investigate the same still. |
Re: functional testing when actually setting in config file or as an env variable: here's the examples we did for the configured endpoint URL. As I recall we did this specifically because this change also introduced complexity with the |
Marking as draft pending review and approval of PR in |
This change allows for host prefix injection to be disabled through an environment variable `AWS_DISABLE_HOST_PREFIX_INJECTION` or a shared configuration file parameter `disable_host_prefix_injection`. This is in addition to the existing Config parameter `inject_host_prefix`. This change also changes the default value of the Config object's `inject_host_prefix` from `True` to `None`. A property is used to preserve the existing behavior when a user does not specify the parameter to set it to `True`. This preserves the behavior of a user setting inject_host_prefix=None while keeping the logic internal.
e764f66
to
5366eda
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## v2 #9268 +/- ##
==========================
==========================
☔ View full report in Codecov by Sentry. |
Issue #, if available:
Port of boto/botocore#3405
Description of changes:
This change adds the ability to disable host prefix injection. It changes the default for
inject_host_prefix
in the Config object to be a True-like objectDEFAULT_TRUE
(fromTrue
) so that the value can be differentiated from a user-set value using the configuration precedence.Testing
Tested with the following commands which uses an operation-specific host prefix:
False
, which is the same as the default behavior:True
, which prevents the host prefix from being prepended to the endpoint URL:I also confirmed the behavior applies when using the configuration file setting
disable_host_prefix_injection
, and even when not using a custom endpoint URL.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.