Re: rgw: feedback on auth engine selection

[Casey Bodley <cbodley@redhat.com>]

On 09/08/2016 01:19 PM, Radoslaw Zarzynski wrote:
> On Wed, Sep 7, 2016 at 11:05 PM, Casey Bodley <cbodley@redhat.com> wrote:
>> Some specific requirements that I recall from our past discussions include:
>>
>> - don't heap allocate auth state per request
> Great. I understood that we don't need to optimize the early
> run-time (setup, initialization) to eradicate dynamic allocations.
> This would allow to make the things easier to read through
> skipping some crazy CRTP/SFINAE stuff.
>
>> - don't construct or call into AuthEngines that shouldn't be enabled for the
>> given handler
> This sounds like a step further in comparison with the current
> solution. In the past we had a lot of "ifs" executed per request.
> Currently, the logic is split into per-engine pieces and encapsulated
> in is_applicable method of AuthEngines. In my understanding
> the goal is to introduce init-time preselection. However, we need
> to discuss dealing with run-time config changes (operator injecting
> new configuration without restart). Do we want to cover this use
> case at all?

Right, this was the requirement that motivated the work on preselection, 
so we would only try to authenticate against engines that are properly 
configured and relevant to the given handler.

With respect to changing the auth strategy (i.e. the list of engines 
configured for a given handler) at runtime, I'm not sure it's worth the 
complexity at this point, and we certainly don't want to add locking to 
do this safely. [note that we do have 'dynamic reconfiguration' which 
pauses the frontend while it reloads RGWRados, so we could use that 
mechanism to change auth strategy without needing new locks]

>
>> I'll add some of the other refactoring goals we've discussed, but I don't
>> think they place any strict requirements on the auth subsystem:
>>
>> - RGWHandler: being able to reuse handler instances, instead of allocating a
>> new one for each request
> At the moment RGWHandler is dynamically allocated on each
> request. Do we plan to remove only the allocations or go even
> further and make handlers entirely state-less?
>
>> - req_state: contains too many unrelated fields, some of which are
>> transformed in confusing ways over the lifetime of the request. needs to be
>> broken up into smaller, better encapsulated sub-objects
> I'm 100% behind that.
>
>> - concurrency: move away from a synchronous process_request() to build on
>> asio frontend work
> Sound really interesting.
>
>
> Let me propose another requirement:
>   - critical components should facilitate unit testing. In the future
>     they should be covered by reasonable set of unit tests.
>
> What do you think?
>
> Regards,
> Radek