deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michal Fojtik ...@mifo.sk>
Subject Re: [PATCH] Openstack driver - adds forced_authentication for /api... checks whether Openstack provider supports buckets collection (is swift service deployed?)
Date Thu, 13 Sep 2012 20:22:25 GMT
On Sep 13, 2012, at 10:18 PM, David Lutterkort <lutter@redhat.com> wrote:

Hi,

Also I found some nasty race-conditions that can occur when two clients
override the Deltacloud.config[].

I have something in progress to make this more nice :)

  -- Michal

> On Wed, 2012-09-12 at 15:32 +0300, marios@redhat.com wrote:
>> From: marios <marios@redhat.com>
>> 
>> related to https://issues.apache.org/jira/browse/DTACLOUD-316
>> https://github.com/ruby-openstack/ruby-openstack/pull/13
>> 
>> Signed-off-by: marios <marios@redhat.com>
>> ---
>> .../deltacloud/drivers/openstack/openstack_driver.rb  | 19 +++++++++++++------
>> server/lib/deltacloud/server.rb                       |  6 +++++-
>> 2 files changed, 18 insertions(+), 7 deletions(-)
>> 
>> diff --git a/server/lib/deltacloud/drivers/openstack/openstack_driver.rb b/server/lib/deltacloud/drivers/openstack/openstack_driver.rb
>> index 5b29840..ca824bd 100644
>> --- a/server/lib/deltacloud/drivers/openstack/openstack_driver.rb
>> +++ b/server/lib/deltacloud/drivers/openstack/openstack_driver.rb
>> @@ -40,6 +40,19 @@ module Deltacloud
>> 
>>         define_hardware_profile('default')
>> 
>> +        def respond_to?(capability)#, credentials)
>> +          if capability == :buckets
>> +            begin
>> +              client = new_client(Deltacloud.config["openstack_creds"], capability)
>> +            rescue Deltacloud::ExceptionHandler::NotImplemented #OpenStack::Exception::NotImplemented...
>> +              return false
>> +            end
>> +            return true
>> +          else
>> +            super(capability)
>> +          end
>> +        end
> 
> This is not a good idea - it means that something as innocuous looking
> as driver.respond_to?(:buckets) has major sideeffects and fail for all
> kinds of reasons.
> 
> The right fix is to change the supported_collections method in
> deltacloud_helper.rb - this whol capability checking seems way too
> convoluted to me, anyway. Why can't we have a
> supported_collections(credentials) method in the base driver which, for
> all I care, use introspection to figure out which collections are
> available, and override that in the OpenStack driver ?
> 
>>         def hardware_profiles(credentials, opts = {})
>>           os = new_client(credentials)
>>           results = []
>> @@ -351,12 +364,6 @@ private
>>           end
>>         end
>> 
>> -        def cloudfiles_client(credentials)
>> -          safely do
>> -            CloudFiles::Connection.new(:username => credentials.user, :api_key
=> credentials.password)
>> -          end
>> -        end
>> -
>> #NOTE: for the convert_from_foo methods below... openstack-compute
>> #gives Hash for 'flavors' but OpenStack::Compute::Flavor for 'flavor'
>> #hence the use of 'send' to deal with both cases and save duplication
>> diff --git a/server/lib/deltacloud/server.rb b/server/lib/deltacloud/server.rb
>> index c7c6c41..4b9485e 100644
>> --- a/server/lib/deltacloud/server.rb
>> +++ b/server/lib/deltacloud/server.rb
>> @@ -44,8 +44,12 @@ module Deltacloud
>>     set :config, Deltacloud[:deltacloud]
>> 
>>     get '/' do
>> -      if params[:force_auth]
>> +      if driver.name == "openstack" or params[:force_auth]
> 
> Why is this check necessary ? The LazyCredentials helper already throws
> a 401 if you try to use credentials and they are not provided.
> 
>>         return [401, 'Authentication failed'] unless driver.valid_credentials?(credentials)
>> +        if driver.name == "openstack"
>> +          Deltacloud.config["openstack_creds"] = credentials
> 
> You can't do that - that's a global object shared across all threads.
> 
>> +          #or here also works: Thread.current["openstack_creds"] = credentials
> 
> This would be better. Though even better would be a direct
> supported_collections(credentials) method on the drivers.
> 
> David
> 
> 


Mime
View raw message