It turns out that eventlet has been injecting a
``Transfer-Encoding`` header as of recent into WSGI application
response headers. The result of this ultimately depends on how
the HTTP client which is passing the request to the server is
written to handle data.
Apache, for example, will return that an invalid response was
received. In part because it sees the request end, with an HTTP
204 response code, but also an encoding indicating there is
a multipart body encoding inbound. Which is confusing.
Other C based HTTP clients can have any number of reactions up to
and including disconnecting sessions. Curl, depending on the
headers present either returns success but notes body weirdness
or actually returns return code 18.
Python-Requests kind of has it a little worse, and we see this
with clients. With it, it tries to prepare a respones content
body based upon the presence of the header indicating there is
a body. But it blows up thinking there is more data to read on
the socket when there is not more data to read.
Regardless, all of this is an RFC7230 violation.
Neither Content-Length nor Transfer-Encoding should be on an HTTP
204 response. However, Content-Length is the lesser evil, and we
have a similar endpoing in Ironic which *does* explicitly get
returned with a zero length content-length, and does not
demonstrate such issues.
As such, in the interest of the lesser evils until Eventlet's evil
ways of header injection are remedied, we're explicitly going to
force a Content-Length header to be sent indicating a zero length
response.
For more information, please see: https://github.com/eventlet/eventlet/issues/746
Change-Id: I014cc65c79222f4d4d7c2b6ff11a76e56659340c
This commit add support for state selector to the list introspection.
* ``GET /v1/introspection?state=[starting, waiting, processing,
finished, error, reapplying,
enrolling]``
Story: 1625183
Task: 11350
Change-Id: I2c5222110487a08a4e7b1efbcbc5dc3d552fae3e
According to the openstacksdk docs[1] the Node uuid is stored in the
id attribute. This change removes the get_node shim which adds the
uuid attribute, and replaces any calls to Node.uuid with Node.id. This
will stop the many log debug warnings about this attribute:
DEBUG openstack.resource [-] Attribute [uuid] not found in
[<openstack.resource._ComponentManager object at 0x7f417e43aa20>]:
'uuid'. __getattribute__ /usr/lib/python3.6/site-packages/openstack/resource.py:623
Calls to NodeInfo.uuid or db.Node.uuid remain unchanged.
Change-Id: Icd3de82877c6a53d32b4c9fd3e500d3cd9d7fb17
Story: 2008379
Task: 41300
SCRIPT_NAME should be used to correctly construct the returned links.
This patch also adds ProxyFix from werkzeug to allow using inspector
behind SSL terminating proxy while still keeping the links correct.
Change-Id: I4f40e9266a55d237f7aa68324b11f59b667cc940
This change implements an alternative middleware which supports the
same deferred auth mechanism as the keystone auth middleware.
When auth fails the header X-Identity-Status is set to Invalid, which
only becomes an Unauthorized response when the path is not public.
Without this change, the paths /, /v1 and /v1/continue
incorrectly require authentication when using basic auth.
Change-Id: I780151870f851ad5dcd45610aacedcca23607a71
Story: 2007656
Task: 39826
When sending a literal empty response, Flask does not include a
ContentType in the response. While in many cases, we don't need
need a ContentType nor expect one on the API client, Apache
webserver can treat this as an error and generate an Error
indicating a Bad Gateway. When doing this, we also now include
an empty JSON body in the response for 202 messages. For 204
message errors, the message body is expected to be empty.
However, when this Bad Gateway error occurs, the API/Conductor
were proceeding like there was no issue. The API client on the
other hand thinks that a hard failure has occured.
Also adds some additional catches to provide additional logging
which turned out not to be needed in this case, but it would be
useful for others.
Change-Id: If2e7697e3fde58ab0a4193787e29d3acdca81ebf
When the config option ``auth_strategy`` is set to ``http_basic`` then
non-public API calls require a valid HTTP Basic authentication header to be
set. The config option ``http_basic_auth_user_file`` defaults to
``/etc/ironic-inspector/htpasswd`` and points to a file which supports the
Apache htpasswd syntax[1]. This file is read for every request, so no
service restart is required when changes are made.
The only password digest supported is bcrypt, and the ``bcrypt``
python library is used for password checks since it supports ``$2y$``
prefixed bcrypt passwords as generated by the Apache htpasswd utility.
To try basic authentication, the following can be done:
* Set ``/etc/ironic-inspector/inspector.conf`` ``DEFAULT`` ``auth_strategy``
to ``http_basic``
* Populate the htpasswd file with entries, for example:
``htpasswd -nbB myName myPassword >> /etc/ironic-inspector/htpasswd``
* Make basic authenticated HTTP requests, for example:
``curl --user myName:myPassword http://localhost:6385/v1/introspection``
[1] https://httpd.apache.org/docs/current/misc/password_encryptions.html
Change-Id: If50dfbfc18445ad9fe27e17cb0ee1b317ff25a0b
Depends-On: https://review.opendev.org/729070
Story: 2007656
Task: 39826
We've been historically using endpoints without trailing slashes in
our API. Apparently, some libraries (like gophercloud) are quite
opinionated about it (see the story), so let's handle both.
The implementation could be simpler if we just added trailing slash
to all routes, but it would cause redirects for current users.
Change-Id: Icbd971a8e792f93f9c3fa66ba29bec055dcdee32
Story: #2007660
Task: #39749
Using autospec ensures that mocked functions are called with correct
arguments, so it's highly desired to have it.
Change-Id: I9c8395adf852495d2ef6db732d727990e8abd5d7
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: Ic67f09a223ae2d0cb460771a10a4122307afa05b
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
Added 'scope' property to IntrospectionRule and logic to check if a node
falls in the same scope.This allows introspection rules to be applied on
selected nodes instead of every one of them.
Story: 2006995
Task: 37763
Change-Id: I77034f032ea0ec16886afdd928546eb801f7a90a
Move ironic-inspector API under apache+uwsgi for the non-standalone job.
Story: 2001842
Task: 30376
Change-Id: I27291003b18beda0a9ddb79d38f615df9e8499ac
This patch splits API and conductor services for ironic-inspector.
Previous patch utilized lock from tooz coordinator, this patch adds
a coordinator wrapper for easier usage and further introduces group
interfaces.
Each conductor service will join a predefined group to mark it's
availability, on each request, API service will query members from
the group and randomly choose on of them, create desiginated topic
and deliver request to it.
The feature is tested with the memcached, file backend of tooz.
Other backends are not fully tested but may work as well, please
refer to tooz documentation for driver compatibilities[1].
[1] https://docs.openstack.org/tooz/latest/user/compatibility.html
Story: 2001842
Task: 30376
Change-Id: I419176cd6d44d74c066db275ef008fe8bb6ef37a
This patch adds support to provide unprocessed introspection data
to reapply a node. The provided introspection data will be save to
current introspection storage backend.
Change-Id: I969ae9c32f53f89c006a64a006388ddea9542aa5
Story: 1564863
Task: 11344
In https://review.openstack.org/#/c/637673, the get introspection
interface is narrowed down to only accept node uuid, which previously
accepts both uuid and name. But the name to uuid conversion is
missing in the reapply api, which causes feature regression, and
this is the fix :(
Story: 1726713
Task: 11373
Change-Id: I6912853deab77f1365f665ca1e52c13063d2cdf1
Configurable introspection data storage backend [1] is proposed
to provide flexible extension of introspection data storage
instead of the single support of Swift storage backend.
This patch adds plugin mechanism for loading introspection
storage, creates database backend and moves Swift storage
into a plugin.
[1] http://specs.openstack.org/openstack/ironic-inspector-specs/specs/configurable-introspection-data-backends.html
Story: 1726713
Task: 11373
Co-Authored-By: Kaifeng Wang <kaifeng.w@gmail.com>
Change-Id: Ie4d09dc0afc441b20a1e5e3bd8e742b1df918954
Adds support to use latest as the microversion value. When set to
latest, the maximum version is assumed by ironic-inspector.
Story: 1672400
Task: 11363
Change-Id: I35be1034697a7d69ed30af9542d9711fb2f65bd0
Adds a new node field "manage_boot" to store this value. When it is set
to False, neither boot device nor power state are touched for this node.
Instead, we expect a 3rd party to handle them.
We still manage the PXE filter because the node may need DHCP.
Change-Id: Id3585bd32138a069dfcfc0ab04ee4f5f10f0a5ea
Story: #1528920
Task: #11338
Adds oslo.messaging to ironic-inspector, and convert
inspect, abort and reapply to synchronized rpc calls.
This is the first step of API and worker seperation.
Change-Id: I15e86d7feb623b6b2889891b9700e5de6b3164cd
Story: #2001842
Task: # 12609
Consolidate all config options under ``conf`` directory.
New config modules should give a better picture of the configuration
options provided by the inspector.
Change-Id: I501ed0787ff4e1d91462f936e1a54de2c7abb35c
Related-Bug: #1561100
Co-Authored-By: Anton Arefiev <aarefiev@mirantis.com>
This experimental feature was deprecated in the Ocata release,
as it was found unstable, untested and dangerous.
API version is bumped to 1.12 to indicate this change to users.
Change-Id: I1aad6ddfd03946edc19ae510accd6c8daf5fc268
Closes-Bug: #1654318
Creates new WSGIService class which keeps base API sercice
initialization functionality and serve flask application. Also
it will configure application for wsgi container[1].
Also creates new `cmd` directory for storing console scripts.
[1] https://governance.openstack.org/tc/goals/pike/deploy-api-in-wsgi.html
Related-Bug: #1525218
Change-Id: Ia64228c47a79a3008d435e8323a964f2bc45dfa7
This adds the node state when the GET /v1/introspection/<node uuid or
name> API is performed.
Change-Id: I81c6834933f789cb644a854313aacaf49a4856a7
Closes-Bug: #1665664
Db column started_at and finished_at are defined as float type, but
float time doesn't fit into default db Float size. This change migrates
columns type to DateTime.
Closes-Bug: 1658975
Change-Id: I13bbb3c9a3e6e8bc744473437a378453ebc81318
This feature is dangerous, barely maintained and not covered by any CI.
As it was hidden behind a configuration option, we can remove it without
breaking our API contract too much. This change deprecates the option,
and create an API version with this feature already de-activated.
Change-Id: I9e05c36b8c1194f4eeeb80c1f811e808854974c4
Partial-Bug: #1654318
This patch introduces an API endpoint to list introspection statuses. The
endpoint supports pagination with an uuid-marker and a limit query string
fields. Due to the pagination, this change introduces a new configuration
option: ``api_max_limit``.
APIImpact
Change-Id: I74d02698801d5290619161b2d8d7181ab51a0a5e
Partial-Bug: #1525238
Enhance the introspection status with the fields:
* uuid
* started_at
* finished_at
Change-Id: I36caa7d954a9bfb029d3f849fdf5e73f06f3da74
Partial-Bug: #1525238
In unittesttools, assertDictEqual() and assertEqual()
are implemented by using '!=' operator. But assertEqual()
can handle dict, list, set and so on. So we just call
assertEqual() to make the tests simpler.
Change-Id: I2fdae6892abd3c66bb0702d518101f501e05c4e9
Oslotest base class should be used as the base for unit test classes.
In addtion the oslotest BaseTestCase inherits from testtools.testCase,
according to fixtures docs "testtools has it’s own implementation of
useFixture so there is no need to use fixtures.TestWithFixtures with
testtools.TestCase."
Co-Authored-By: moshe levi <moshele@mellanox.com>
Change-Id: I7296d59ee30230ec2de6d55649c9f57a33534435
Before https://review.openstack.org/#/c/337043/,
stevedore.NamedExtensionManager returned KeyError
when calling with non-existing names and name_order=True.
After the mentioned change this is not longer true,
so we used the just added on_missing_entrypoints_callback option
to customize the behavior in this cases and make it raise
a custom exception MissingHookError.
Change-Id: I1f1edc0b7a82a16bf9be4113db61ee1cd0080db4
Closes-Bug: #1600141
https://review.openstack.org/#/c/337043/ makes this test to fail,
but proper fix in https://review.openstack.org/339457 requires
release of stevedore > 1.15.0 not released yet.
This patch skips the test temporarily until new release is added
in global requirements.
Change-Id: Id23efad9c392fc70470d996e37d378efeaf65491
Partial-Bug: #1600141
This change introduces new return code (201 instead of 200) for
POST /v1/rules endpoint on success rule creation.
API less 1.6 continues returning 200.
Default API version was changed from minimum to maximum
which Inspector can support.
Change-Id: I911c7c241d16b9948ee4b6db92b127c7f8f374ae
remove iterated form of side effects from some test cases
that are simulating single exception/error
Change-Id: I5e9cb760587a48d8bbe059191f3605f6ed547a44
Closes-Bug: #1564392
This change drops check on UUID validness from our API.
It also has a subtle effect of doing Ironic node fetching in
the introspection status and data fetch calls, which might make them
slightly longer (but only when name is used).
A new helper common.ironic.get_node is created to unify how we fetch nodes
from Ironic. It also provides nicer exceptions.
Change-Id: I20cf65e57910568b70a62c3f9269a962e78a07e2
Closes-Bug: #1523902
As requested in the related bug, this pull request allows to run the
introspection again on previously stored data. This should make it simple
to correct mistakes in the introspection rules.
For this purpose, new API entry point was introduced:
/v1/introspection/<UUID>/data/unprocessed that supports an empty POST
method to trigger the introspection over stored data. New function
`reapply` was introduced that takes care about the entry point and carries
out the introspection. The `process` function was modified to allow
reusing common parts in the new reapply function. The storage access
methods were updated to allow saving the "raw" memdisk data besides the
processed introspection data.
Following preconditions are checked the reapply function having been
triggered:
* no data is being sent along with the request
* Swift store is configured and enabled and the stored data is present
for the node UUID
* node_info object is cached for the UUID and it is possible to lock the
object
Should the preconditions fail, an immediate response is given to the user:
* 400 if the request contained data or in case Swift store is not enabled
in configuration
* 409 if it wasn't possible to acquire lock for the node_info object
* 404 in case Ironic didn't keep track of related BM node
If the preconditions are met, a background task is executed to carry out
the processing and a 202 Accepted response is returned to the endpoint
user.
As requested, these steps are performed in the background task:
* preprocessing hooks
* post processing hooks, storing result in Swift
* introspection rules
These steps are avoided, based on the RFE:
* not_found_hook is skipped
* power operations
Limitations:
* IMPI credentials are not updated --- ramdisk not running
* there's no way to update the raw data atm.
* the raw data is never cleaned from the store
* check for stored data presence is performed in background;
missing data situation still results in a 202 response
Change-Id: Ic027c9d15f7f5475fcc3f599d081d1e8d5e244d4
Closes-Bug: #1525237
This refoctor is needed for tempest test work as tempest tests
will placed in test dir. So move unit tests to separate directory
"unit" under test.
Change-Id: Ic99df6111ef30947148a9e38b9435a54f3d37064
cid