Open Stack

 ---- On Thu, 28 Nov 2019 08:34:03 -0600 Matt Riedemann <mriedemos at gmail.com> wrote ----
 > On 11/27/2019 12:26 PM, Ghanshyam Mann wrote:
 > > In old change where we added the verify_project_id did not change the success case, that only handled the case where keystone returned
 > > 404 on GET /project means it is confirmed that the requested project does not exist so it will break later so nova started 400 instead of 200.
 > 
 > What "old change" are you referring to here? Before [1] there was *no* 
 > validation performed when adding/removing flavor access or updating 
 > quotas for a given project. Hence all of the duplicate bugs about being 
 > able to typo/fat-finger/pass garbage values to those APIs and then be 
 > confused later when flavor access and quotas aren't working as expected. 
 > There have been a few changes to the verify method since it was added, 
 > but the point is it was a non-microversion change to admin APIs which 
 > would turn previously invalid but passing requests to failures.

+1. agree on that change which is making an invalid passed request to failure so does not require microversion. 

 > 
 > I'm assuming 403 was handled "gracefully" more for backward 
 > compatibility than anything else but as can be seen from Surya's bug is 
 > just masking the original issue that this validation code was trying to fix.
 > 
 > Nothing is changing in request or response schema and this should only 
 > be enforced (re-raise on 403 from keystone) in the APIs that are 
 > admin-only by default so it's not an interop concern.

This is a good point, On rethinking on "how the operator can fix the current success case which 
going to be failed case after proposed solution" is via a configuration change only (change the policy permissions).
Which is similar to changing the default roles for any policy. If we change the policy defaults then we follow the
deprecation phase only and do not bump the microversion. If we can follow the deprecation warning (the same as a policy
change case) in this proposed solution also then we can avoid the microversion bump. 

I mean, in Ussuri release, we log the warning saying that "this API might be successful for you but it will fail if the user does not have
GET /project" permissions from V release. and in V release we raise the error for keystone's 403 or non-2xx cases also. 


-gmann

 > 
 > I just don't see why we would expect someone to need to opt into this 
 > validation actually working - and if misconfigured actually failing to 
 > indicate to the admin using the API that their deployment needs to be fixed.
 > 
 > [1] https://review.opendev.org/#/c/435010/
 > 
 > -- 
 > 
 > Thanks,
 > 
 > Matt
 > 
 >