Lock, migrate, and unshelve server actions don't enforce request body schema for certain microversions

[Artom Lifshitz]

Description
===========
Basically $summary. For lock, migrate, and unshelve, we have decorators for validation schema that _start_ at a certain microversion (exact microversion varies), meaning anything below that is not checked. A client could send a request that is only valid in higher microversion, omit sending a microversion (probably by mistake), and be surprised when the request is accepted but not honoured.

Steps to reproduce
==================
1. Send a request with random stuff in the body
ex:

curl -g -i -X POST http://10.0.77.83/compute/v2.1/servers/a45ae810-89ef-44fb-b751-013a8740647b/action \
  -H "Accept: application/json" \
  -H "Content-Type: application/json" \
  -H "User-Agent: python-novaclient" \
  -H "X-Auth-Token: <snip>" \
  -H "X-OpenStack-Nova-API-Version: 2.1" \
  -d '{"lock": {"foo": "bar"}}'

OR

  -d '{"migrate": {"foo": "bar"}}'

OR

  -d '{"unshelve": {"foo": "bar"}}'

Expected result
===============
400 Bad Request (or similar)

Actual result
=============
HTTP/1.1 202 Accepted

Environment
===========
Reproduced on master with devstack+kvm. Originally reported on wallaby https://bugzilla.redhat.com/show_bug.cgi?id=2172851

Additional info
===============
I (manually, so there could be errors) went through the code, and those are the only 3 instances of this that I found. Every other API controller method correctly validates its request body across the entire range of the microversions where it's supported.

[Uggla (rene-ribaud)]

Bug verified, minimum API version not set. --> set it to confirmed.

[Artom Lifshitz (notartom)]

Fix Committed is for when a fix is merged, and is set by automation.

[Ghanshyam Mann (ghanshyammann)]

Right, that is expected behaviour as per the backward compatibility. If microversion is not sent (intentionally or mistakenly), then request will be consider as v2.1 request and any validation/schema/processing is there for v2.1 will be done by nova.

Now talking about Nova action APIs, I agree that they are not good interfaces and we did not have any schema validation for 2.0/2.1. For example lock server API, 'lock' in the request body (<2.73 microversion) is just used to select which action to be called and its body value is ignored completely.

That is why anything is allowed: {“lock”: null}, {“lock”: {}}, {“lock”: {'foo': 'foo'}}.

If we add schema to disallow those dummy/empty values then it is a backward incompatible change and cannot be done without microversion bump. Otherwise, users/scripts using it like {“lock”: {}} will start failing.

Another example of such change was to make additionalProperties False and we did that with microversion bump only (2.75).

One opportunity to fix these request are when we change any of these action APIs for example 2.73 in lock APIs, while adding the new data field in request, we do allow only {“lock”: null} or {“lock”: (<expected field of that microversion>)} and disallow all other empty object or dummy values ({“lock”: {}}, {“lock”: {'foo': 'foo'})

[Jorge San Emeterio (jsanemet)]

So, what I gather is that the behavior described on this ticket is expected and should be left as is, right?

[Artom Lifshitz (notartom)]

Well, it was still a _mistake_, but it's been out in the wild long enough that we can't retroactively fix it without breaking someone's scripts, so we have to leave it as is, unfortunately :(

[OpenStack Infra (hudson-openstack)]

Change abandoned by "Jorge San Emeterio <email address hidden>" on branch: master
Review: https://review.opendev.org/c/openstack/nova/+/875653
Reason: Unfortunately, the behavior changed here will break things retroactively and thus is not feasible. The bug ticket has been declared as 'Won't Fix'.