I tried the new version of the cinder patch and it's working well from the nova point of view.
The new check for the service role prevents the X-Service-Token header bypass and there should not be any way to fake the roles because the roles on the RequestContext are extracted only from a validated token response from keystone which will return the roles internally associated with the token. (I tried sending my own X-Service-Roles header and it [correctly] did not work).
Other than that, upon review I noticed there are a few typos in the unit tests in the patch, for example "mock_action_get.assesrt_called_once_with". Because the mocks are MagicMocks this will be a mocked function call that doesn't check anything and will not fail.
I also got one unit test fail when I ran them (test__redirect_detach_to_nova_if_needed*) locally but it's possible that's something unrelated in my environment.
Hi Gorka,
I tried the new version of the cinder patch and it's working well from the nova point of view.
The new check for the service role prevents the X-Service-Token header bypass and there should not be any way to fake the roles because the roles on the RequestContext are extracted only from a validated token response from keystone which will return the roles internally associated with the token. (I tried sending my own X-Service-Roles header and it [correctly] did not work).
Other than that, upon review I noticed there are a few typos in the unit tests in the patch, for example "mock_action_ get.assesrt_ called_ once_with" . Because the mocks are MagicMocks this will be a mocked function call that doesn't check anything and will not fail.
I also got one unit test fail when I ran them (test__ redirect_ detach_ to_nova_ if_needed* ) locally but it's possible that's something unrelated in my environment.
Thank you for fixing up the patch so fast!