When redirected, the server *generally* returns a fully
formed URI, but does not really have to, so we may end up
in a "depending on how the redirect was triggered" would
result in the response handling.
Ultimately, any behavior which is not an fully formed URI
would be invalid.
But our code was taking the URI we got back, and would then
re-issue the request with a list of parameters with the new
URL. Duplicating the parameters on the URI.
Example of what was occuring, when only provision_state=active
was a parameter before the redirect:
/v1/nodes?provision_state=active&provision_state=active
Co-Authored-By: Kristi Nikolla <knikolla@bu.edu>
Co-Authored-By: Jay Faulkner <jay@jvf.cc>
Story: 2010029
Task: 45316
Change-Id: I4969a42ee651ac2c559e378d879b673a1d788c57
Currently when redirects are used, the request id can get lost on redirect.
Proposed patch reuses req-id from redirect response and passes it to
actual request
Closes-Bug: #2000742
Change-Id: I98d5d4490b3d5667677cdd19f3c7b39abe6044ef
This is no longer necessary since we only support Python 3.x.
A note is removed from requirements.txt since it's no longer relevant:
pip 20.3+ has a real dependency resolver.
Change-Id: Ie3006813a79fef1f128d388b906e4f1752347fa4
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Co-Authored-By: Grzegorz Grasza <xek@redhat.com>
Noticed this while doing some local testing, if a WSGI app replies with
a text/plain content type to communicate a server error, we aren't able
to see the error response message when passing --debug to the
openstackclient, example:
RESP: [500] Date: Thu, 01 Oct 2020 23:54:15 GMT Server: Apache/2.4.18
(Ubuntu) Content-Type: text/plain; charset=UTF-8 Connection: close
Transfer-Encoding: chunked
RESP BODY: Omitted, Content-Type is set to text/plain; charset=UTF-8.
Only application/json responses have their bodies logged.
Change-Id: Ibfd46c7725bd0aa26f1f80b0e8fc6eda2ac2e090
Manila API honors a "X-OpenStack-Manila-API-Version"
header to specify microversions.
It may support the OpenStack-API-Version header
in a future release, however, we'll need to maintain
backwards compatibility with the existing API.
Change-Id: Ia2e62d3a11a08adeb6d488b7c9b365f7ff2be3c8
In case of global-request-id request, Adapter
send two global request id header
- "X-OpenStack-Request-ID"
- "X-Openstack-Request-Id".
Example: https://zuul.opendev.org/t/openstack/build/c5b1debf78df4aa3bdda34f0b4c53c37/log/testrepository.subunit#2385
This is becasue of the header not being Case Insensitive
and end up with two different name of same header with difference
of cap 'D'.
Unit test for whether request global-request-id has precedence
over adapter fail many times because of how different python version
treat the dict. py3.6 and above are all good as dict maintain the
insertion ordered but py3.5 can fail it any time.
We can see consistent failure in py35 jobs:
- https://review.opendev.org/#/c/730687/
Let's make the headers always Case Insensitive which is
what RFC says.
Change-Id: Iba707dd0506d22e144aca4fdfc9b140c8e37ae02
Closes-Bug: #1881351
Now that we no longer support py27, we can use the standard library
unittest.mock module instead of the third party mock lib.
Change-Id: I07d61e1a8f18d65acdf86cdd61f7d9e28157f1d7
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
With the requests-mock logger now configured to log the request[1],
checking that the logger output does *not* contain the request is
invalid. Simplify these two tests by omitting the assertion.
[1] https://github.com/jamielennox/requests-mock/pull/93
Closes-bug: #1842978
Change-Id: If3c0447502917bce831d3e9f7ae4c31374dd4380
Though we can now set ``connect_retires`` while creating an adapter object,
that would allow retries in case of connection timeout (ex. with session
clients derived from Adapater/LegacyJsonAdapater), it can't be used in
certain scenarios like endpoint discovery with auth plugin get_discovery()
or getting AccessInfo with get_access()/get_auth_ref().
Having ``connect_retries`` in Session constructor would allow users
with option of setting it when creating session objects (if they want)
and can be overridden per service with the adapter interface.
This commit also changes the default value of ``connect_retries`` from
0 to None to allow for adapter's to override retries on the session
object.
Depends-On: https://review.opendev.org/#/c/680497/
Change-Id: Iffb671fefae23926b1f09017d9db438341eae238
Partial-Bug: #1840235
If an external session object was not passed to the Session class, we
create a requests.Session() on our own. Once this is used, it may still
have an open connection when the auth Session is closed. We need to
handle the closing of the requests.Session() ourselves if we created
one. If you do not close it, a ResourceWarning may be reported about the
socket that is left open. If a session object is provided, we do not
attempt to close it as it will be up to the code consuming keystoneauth
to properly handle cleaning up the provided session.
Change-Id: I590755d665b371c76ba8e02836d81d41a95ac601
Closes-Bug: #1838704
Clients like ironicclient and swiftclient use fixed delay for their
build-in retry functionality. To replace it without changing behavior
we need a similar feature.
Change-Id: I1f9de98dae5719842f03d45e5a9d724199d5718b
Adapter.__init__ takes a global_request_id which causes the
X-Openstack-Request-Id header to be set on each request. This is fine if
the Adapter is used for only one "request" (in the sense of e.g. "a
server create" -- see [1]), but is too broad if the Adapter is reused
for multiple requests. For example, Nova's SchedulerReportClient (used
to communicate with Placement) creates a single instance of Adapter for
the life of the process [2][3][4]. Openstack SDK's Proxy objects [5]
endure for the life of a Connection.
So what is needed is a way to manage the X-Openstack-Request-Id header
on a per-request basis.
This commit adds a global_request_id kwarg to
keystoneauth1.session.Session.request, which is the funnel point for all
requests coming through Adapter as well as Session itself. (All the
methods feeding into that one already accept and pass through arbitrary
**kwargs.) If present, the value in the X-Openstack-Request-Id header is
set accordingly. Note that this will *override*
Adapter.global_request_id, which is exactly what we want, as described
above.
[1] http://specs.openstack.org/openstack/oslo-specs/specs/pike/global-req-id.html
[2] bea9058f02/nova/scheduler/client/report.py (L200)
[3] bea9058f02/nova/scheduler/client/report.py (L243)
[4] bea9058f02/nova/utils.py (L1219-L1221)
[5] bf6651f149/openstack/proxy.py (L114)
Change-Id: Ied73320fcd813ae796e40cbdb30717900486b92c
Currently it grows exponentially, exceeding 1 hour after 15 retries.
While we don't expect people to have so many retries, we should not
let them shoot their legs.
Change-Id: I01dfaa1c379340a0d41fcfdb07298fdef6110941
shade/openstacksdk has implemented client-side rate limiting on top of
keystoneauth for ages and uses it extensively in nodepool. As part of an
effort to refactor that code a new approach was devised which was much
simpler and therfore suitable for inclusion in keystoneauth directly.
The underlying goal is two-fold, but fundamentally is about allowing a
user to add some settings so that they can avoid slamming their cloud.
First, allow a user to express that they never want to exceed a given
rate. Second, allow a user to limit the number of concurrent requests
allowed to be in flight.
The settings and logic are added to Adapter and not Session so that the
settings can easily be per-service. There is no need to block requests
to nova on a neutron rate limit, after all.
Co-Authored-By: Ian Wienand <iwienand@redhat.com>
Needed-By: https://review.openstack.org/604926
Change-Id: Ic831e03a37d804f45b7ee58c87f92fa0f4411ad8
With the recent Bandit update[0], the usage of SHA1 is now being
tagged as an issue. This changes the hashing of logs to SHA256
instead of SHA1.
Change-Id: Icde62b8d5ff78b4155e9df8231d63be3ecc53520
It can be annoying to have to say raise_exc=False (or use try/except) on
every call when talking to an API where 4xx response codes are
useful/normal/informative or where the preferred coding style is to use
conditionals rather than try/except.
With this change, the Adapter constructor takes a new kwarg, raise_exc.
It defaults to None, and the existing behavior is unchanged. If set to
a boolean value, that is used as the default for requests. Specifying
raise_exc to the primitives (get, head, put, post, patch, delete,
request) at any point along the chain will still take precedence.
Change-Id: Ie291c3cb891467728d8ca33cf62afdab37c82f34
Closes-Bug: #1776501
Ironic commonly returns HTTP 409 when a node is locked by another routine
and HTTP 503 when the conductor has no free threads to process the request.
Currently it is managed by custom code in ironicclient and openstacksdk,
this change will allow to move it to Session itself.
Change-Id: I04e356e7856b020cd20aa598e291ef31e02730d2
python-openstackclient does this in a wrapper class around Session,
and openstacksdk does something similar that could be removed if support
were directly in keystoneauth.
Add this so that we can remove the custom wrapper/manipulation in
openstackclient and openstacksdk.
Change-Id: Icf00c66f57d20d2cef724c233160d3b1e0d52102
A change introduced in 3.5.0 sorts headers, but runs into a problem
when the headers are bytes, such as the headers provided by the
python-glanceclient.
requests expects headers to be str type in both python2 and python3.
This means in python2 we need to encode unicode objects as ASCII (the
encoding that should be used for HTTP headers) and in python3 we need to
decode bytes as ASCII into str.
Change-Id: Ib81497c3a873616c22ba68256c596a6fb113e11e
Closes-bug: #1766235
Python logging is pretty amazingly flexible, and allows us to emit
to arbitrary logging domains so that a consumer can direct log output
with specificity.
Turning on HTTP debug logging currently produces an avalanche of output,
when sometimes just seeing that the requests were made and responded to
is perfectly fine.
Split the loggers used into four - one for request ids, one for request
commands, one for response headers and one for response body content.
Make them subloggers of keystoneauth.session so that if a user does nothing,
their existing logging config will be unchanged.
If someone passes in a logger, behave as before logging all things to
the provided logger.
While we're at it, document this in the using-sessions document, so that
people know that the loggers exist and what they do.
NOTE:
The tox (>=1.7.0) by default sets a random python hash seed which
causes ordering of dicts and sets to be different between tests runs.
Disabled the random python hash seed by setting PYTHONHASHSEED=0 to
fix the random failure of below test:
keystoneauth1.tests.unit.test_session.SessionAuthTests.
test_split_loggers
The PYTHONHASHSEED=0 is removed in the followup patch so that we can
separate the tracking down of ordering issues in tests from this patch.
Change-Id: Ide7dac8adf5c76c9019c35867cda632aff39770f
The additional_user_agent is intended for things, such as
os-client-config, that sit somewhere in the stack between keystoneauth
and the actual client. So according to jamielennox in irc, it should be:
app client *additional_user_agent keystoneauth requests
The current logic puts additional_user_agent between app and client,
leading to things that look like this:
"User-Agent: v.py/1 os-client-config/1.26.1 shade/1.19.1
keystoneauth1/2.18.0 python-requests/2.13.0 CPython/2.7.12"
Which is a bit off.
Change-Id: Iddfbeab322a39d3ba893a15aabb4c79050526451
The previous microversion patch had some review comments from samuel and
colleen that this addresses. Also, add a release note.
Change-Id: Id83643ee5a00abc5134a88dfa5bc8ddb4f5a247a
We're discouraging the use of the ambiguous and difficult-to-understand
'version' parameter in new discovery methods, instead encouraging the
use of min_version and max_version.
In order to make it possible to get the same functionality, though, we
need a way to say the same thing as version="M.m", which actually means,
"min version is M.m, and max version is the latest within major version
M".
Introducing 'latest' syntax, which can be used in various ways,
including:
min_version='2.3', max_version='2.latest'
...which is equivalent to the old school version='2.3'
Change-Id: Ife842333e25c33e54bbae4c1adb101014cb8e8db
The user now has the ability to know what microversions are available,
but needs to be able to send a microversion header with their request.
Add a microversion parameter to Session that will construct and send the
header. The microversion header requires a service_type. One should be
available but it's possible for it to be missing if someone is using an
endpoint_override. Provide a parameter to let the user specify a
service_type for the microversion call in such cases.
Change-Id: I63cdd67701749630228f9496eda82b3c8747a608
In order to make it easier for projects to enable global_request_id
passing, make it something the Adapter understands directly, so that
the logic of adding extra headers doesn't need to be in every client.
Part of the push to enable global_request_id throughout OpenStack.
oslo spec I65de8261746b25d45e105394f4eeb95b9cb3bd42
Change-Id: Ic75be3acb8b77aae8da631e3c4cd6f545a9a35cb
A response with header Content-Type set to "application/json; charset=UTF-8"
would be omitted but not correctly logged. This patch set correctly omits
and logs a response with the mentioned header.
Co-Authored-By: Tin Lam <tinlam@gmail.com>
Change-Id: I21a185db4ca55ff16dba60f85bb229ffdacc2afa
Closes-Bug: #1656981
When whitelisting content types to debug print from session we chose
application/json and application/text. application/text is not a real
mime type, text is typically text/plain.
Rather than guess at mime types only print application/json to start
with, but make it easy for additional types to be added later.
Change-Id: Ica5fee076cdab8b1d5167161d28af7313fad9477
Related-Bug: 1616105
Currently, logs display the hash values of X-Auth-Token,
Authorization, and X-Subject-Token, but not the value of
the X-Service-Token. This patch set adds the X-Service-Token
to the list of header fields to be hashed for logging purposes.
Change-Id: I4d996a2631f61a2c9bbbc7f959e97c7279be023d
Closes-Bug: #1654847
Added support to log 'X-Openstack-Request-Id' or
'X-Compute-Request-Id' in case of Nova for each api call.
If any python-*client which is using session is used from
command line then following log will be logged on console
if --debug flag is used.
DEBUG (session:350) GET call to compute for
http://10.232.48.201:8774/v2.1/servers/detail used
request id req-c54b8f3e-a7e4-4085-a5e3-fd5244ef3ce5
If any python-*client which is using session is used in
applications (e.g. Nova) then following log message will be
logged in service logs.
DEBUG keystoneauth.session
[req-a6929d46-765c-44a9-8370-49ff0f1958ca admin admin] GET call
to network for
http://10.232.48.201:9696/v2.0/security-groups.json?id=040cc729-9086-4f41-8977-acb4ef71c7de used request id req-de6bfe07-22ac-4940-b65e-367cb0e8102d
DocImpact:
To use this feature user need to set 'default_log_levels' in third
party application. For example in nova.conf 'default_log_levels'
should be set as below:
default_log_levels = keystoneauth1=DEBUG
Closes-Bug: #1605488
Change-Id: If0c5a4eb7d51c601ba38149f846ebcd6116d18be
Response bodies are loaded into memory prior to
being logged.
Loading huge response bodies may result in a
MemoryError.
This patch proposes that only JSON and TEXT
responses be logged, i.e when the Content-Type
header is application/json or application/text.
Responses that do not include or have a different
Content-Type header will have their body omitted.
Closes-bug: 1616105
Change-Id: I93b6fff73368c4f58bdebf8566c4948b50980cee
1.As mentioned in [1], we should avoid using
six.iteritems to achieve iterators. We can
use dict.items instead, as it will return
iterators in PY3 as well. And dict.items/keys
will more readable. 2.In py2, the performance
about list should be negligible, see the link [2].
[1] https://wiki.openstack.org/wiki/Python3
[2] http://lists.openstack.org/pipermail/openstack-dev/2015-June/066391.html
Change-Id: I9f8f2c35f0d45d866076507a3a167aaafb8382e5
Auth token middleware does a bit of a hack where it passes an Adapter in
as a session to the client. This is useful there because we need to know
much more about the authentication information than we do in most
clients.
We should look at fixing this in auth_token middleware, however for now
we shouldn't issue a deprecation warning when a user passes an Adapter
as a session object because this has always been designed to work - just
not something we recommend.
Change-Id: If7ebe59d5908275e607f32244027c8e6f3d1e157
Closes-Bug: #1647230
You can pass client_name and client_version to Adapter.__init__ but for
most clients this means overriding Adapter.__init__ and setdefault()-ing
the client_name and version.
As most clients already override the Adapter object it'd be easier if
they could just set these values on the class as they are not going to
change between instances.
Change-Id: I301a7f77c8cf423bc1d45e3dcbb2325f6853b9a9
Now if body contains json, its content type forcibly
changes to application/json, which is not correct in
some case, like json-patch request.
This code fixes this situation and sets application/json
contnent type only if this header hasn't been set before.
Change-Id: I4e0c44d444519f056dfa48c9603dbc3ca6b01822
Closes-bug: #1634110
Allow specifying a service name and version to the session and a client
name and version to the adapter. The way this will work is that
libraries such as keystoneclient will pass client_name and
client_version when creating their adapter. Then when nova or another
service creates a session it will provide the service name and version.
The combination of these will be used to provide a meaningful user
agent.
Change-Id: Ibe516d9b248513579d5e8ca94015c4ae9c00f3f9
Closes-Bug: #1614846
The Windows Subsystem for Linux is not a complete implementation
of the Linux APIs, and setting TCP_KEEPCNT is currently
unimplmenented. Attempting to use this option will cause HTTP
connections to fail. This change checks if we are running under
WSL, and disables changing TCP_KEEPCNT parameters if so.
Change-Id: Ic8b41dea2a75660d9adbce88a00a0fe703a4d120
Closes-Bug: #1614688
If the application name, sys.argv[0], is an empty string, like when
called from the Ansible os_network module, then keystoneauth (with
the new requests 2.11.0) will raise an exception saying:
Invalid return character or leading space in header: User-Agent
This comes from keystoneauth trying to prepend the empty module
name to the default user agent, which leads to it sending a
User-Agent header that starts with a blank space. This was fine
until the new requests 2.11.0 was released which errors if header
values start with a leading space.
With this change, if sys.argv[0] is an empty string we'll just use
the DEFAULT_USER_AGENT and not try to prepend a calling module.
Closes-Bug: #1611426
Change-Id: I56d3e352dce7628add0479b3333a880700844ebc
Signed-off-by: Matt Mulsow <mamulsow@us.ibm.com>
There was no test that verified that when sys.argv is an empty
list or when sys.argv[0] is an empty string that the default
user-agent value is constructed as expected.
Change-Id: Iacf1557b6dbf48bf6bfb59d7cfbfec14e4fa8e56
Related-Bug: 1611426
Julia Kreger