You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by lu...@redhat.com on 2011/04/19 00:34:58 UTC

Fix critical EC2 error

The current EC2 driver contains a bug that makes most requests fail with an
invalid signature. The issue, reported by jayg, was introduced in commit
b8e725c6

These patches also improve the Eucalyptus driver to not modify environment
variables.

Unfortunately, cucumber and server unit tests fail for me, even on the
trunk, so I've only tested these patches manually.

David

Re: Fix critical EC2 error

Posted by Sang-Min Park <sa...@eucalyptus.com>.
Ok, it was a bug that I introduced and I'm sorry about that :(

It should have been
 ENV['EC2_URL']=nil
 ENV['S3_URL']=nil
instead of
ENV['EC2_URL']=''
ENV['S3_URL']=''


Sang-min



On Mon, Apr 18, 2011 at 3:34 PM, <lu...@redhat.com> wrote:

>
> The current EC2 driver contains a bug that makes most requests fail with an
> invalid signature. The issue, reported by jayg, was introduced in commit
> b8e725c6
>
> These patches also improve the Eucalyptus driver to not modify environment
> variables.
>
> Unfortunately, cucumber and server unit tests fail for me, even on the
> trunk, so I've only tested these patches manually.
>
> David
>

Re: [PATCH 1/2] Eucalyptus: do not modify env variables EC2_URL and S3_URL

Posted by Sang-Min Park <sa...@eucalyptus.com>.
Thanks David. I manually tested it against Eucalyptus back-end and the patch
works fine.
I did also test against EC2 and it works too.

Sang-min

On Mon, Apr 18, 2011 at 3:34 PM, <lu...@redhat.com> wrote:

> From: David Lutterkort <lu...@redhat.com>
>
> Using environment variables to set the Eucalyptus endpoints is inherently
> racy and requires that the EC2 driver knows what the Euca driver is up to.
>
> We now set the Eucalyptus endpoint when we construct the client, for each
> client and thereby avoid any global changes.
> ---
>  .../drivers/eucalyptus/eucalyptus_driver.rb        |   41
> ++++++++++----------
>  1 files changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/server/lib/deltacloud/drivers/eucalyptus/eucalyptus_driver.rb
> b/server/lib/deltacloud/drivers/eucalyptus/eucalyptus_driver.rb
> index 27756ab..c94dc6a 100644
> --- a/server/lib/deltacloud/drivers/eucalyptus/eucalyptus_driver.rb
> +++ b/server/lib/deltacloud/drivers/eucalyptus/eucalyptus_driver.rb
> @@ -134,32 +134,31 @@ module Deltacloud
>                                          "Loadbalancer",
>                           "Loadbalancer not supported in Eucalyptus", "")
>                   end
> -          klass.new(credentials.user, credentials.password,
> eucalyptus_endpoint)
> +          klass.new(credentials.user, credentials.password,
> +                    endpoint_for_service(type))
>         end
>
> -        def eucalyptus_endpoint
> +        SERVICE_STUBS = {
> +          "ec2" => "/services/Eucalyptus",
> +          "s3" => "/services/Walrus"
> +        }
> +
> +        DEFAULT_PORT = 8773
> +
> +        def endpoint_for_service(service)
> +          service = service.to_s
>           endpoint = (Thread.current[:provider] || ENV['API_PROVIDER'])
> -          if endpoint && (endpoint.include?('ec2') ||
> endpoint.include?('s3'))   # example endpoint: 'ec2=192.168.1.1;
> s3=192.168.1.2'
> -            default_port=8773
> -            endpoint.split(';').each do |svc_addr|
> -               addr = svc_addr.sub('ec2=','').sub('s3=','').strip
> -                if addr.include?(':')
> -                   host = addr.split(':')[0]
> -                   port = addr.split(':')[1]
> -                else
> -                   host = addr
> -                  port = default_port
> -                end
> -                if svc_addr.include?('ec2')
> -                  ENV['EC2_URL'] = "http://
> #{host}:#{port}/services/Eucalyptus"
> -                elsif svc_addr.include?('s3')
> -                   ENV['S3_URL'] = "http://
> #{host}:#{port}/services/Walrus"
> -               end
> -             end
> -             {}
> +          if endpoint && endpoint.include?(service)
> +            # example endpoint: 'ec2=192.168.1.1; s3=192.168.1.2'
> +            addr = Hash[endpoint.split(";").map { |svc|
> svc.strip.split("=") }][service]
> +            host = addr.split(':')[0]
> +            port = addr.split(':')[1] || DEFAULT_PORT
> +            stub = SERVICE_STUBS[service]
> +            { :endpoint_url => "http://#{host}:#{port}#{stub}",
> +              :connection_mode => :per_thread }
>           else
>             #EC2_URL/S3_URL env variable will be used by AWS
> -            {:connection_mode => :per_thread}
> +            { :connection_mode => :per_thread }
>           end
>         end
>       end
> --
> 1.7.4.4
>
>

[PATCH 1/2] Eucalyptus: do not modify env variables EC2_URL and S3_URL

Posted by lu...@redhat.com.
From: David Lutterkort <lu...@redhat.com>

Using environment variables to set the Eucalyptus endpoints is inherently
racy and requires that the EC2 driver knows what the Euca driver is up to.

We now set the Eucalyptus endpoint when we construct the client, for each
client and thereby avoid any global changes.
---
 .../drivers/eucalyptus/eucalyptus_driver.rb        |   41 ++++++++++----------
 1 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/server/lib/deltacloud/drivers/eucalyptus/eucalyptus_driver.rb b/server/lib/deltacloud/drivers/eucalyptus/eucalyptus_driver.rb
index 27756ab..c94dc6a 100644
--- a/server/lib/deltacloud/drivers/eucalyptus/eucalyptus_driver.rb
+++ b/server/lib/deltacloud/drivers/eucalyptus/eucalyptus_driver.rb
@@ -134,32 +134,31 @@ module Deltacloud
                                          "Loadbalancer",
                           "Loadbalancer not supported in Eucalyptus", "")
                   end
-          klass.new(credentials.user, credentials.password, eucalyptus_endpoint)
+          klass.new(credentials.user, credentials.password,
+                    endpoint_for_service(type))
         end
 
-        def eucalyptus_endpoint
+        SERVICE_STUBS = {
+          "ec2" => "/services/Eucalyptus",
+          "s3" => "/services/Walrus"
+        }
+
+        DEFAULT_PORT = 8773
+
+        def endpoint_for_service(service)
+          service = service.to_s
           endpoint = (Thread.current[:provider] || ENV['API_PROVIDER'])
-          if endpoint && (endpoint.include?('ec2') || endpoint.include?('s3'))   # example endpoint: 'ec2=192.168.1.1; s3=192.168.1.2'
-	     default_port=8773
-	     endpoint.split(';').each do |svc_addr|
-	        addr = svc_addr.sub('ec2=','').sub('s3=','').strip
-                if addr.include?(':')
-                   host = addr.split(':')[0]
-                   port = addr.split(':')[1]
-                else
-                   host = addr
-	           port = default_port 
-                end
-                if svc_addr.include?('ec2')
-	           ENV['EC2_URL'] = "http://#{host}:#{port}/services/Eucalyptus"
-                elsif svc_addr.include?('s3')
-                   ENV['S3_URL'] = "http://#{host}:#{port}/services/Walrus" 
-	        end
-             end 
-             {}
+          if endpoint && endpoint.include?(service)
+            # example endpoint: 'ec2=192.168.1.1; s3=192.168.1.2'
+            addr = Hash[endpoint.split(";").map { |svc| svc.strip.split("=") }][service]
+            host = addr.split(':')[0]
+            port = addr.split(':')[1] || DEFAULT_PORT
+            stub = SERVICE_STUBS[service]
+            { :endpoint_url => "http://#{host}:#{port}#{stub}",
+              :connection_mode => :per_thread }
           else
             #EC2_URL/S3_URL env variable will be used by AWS
-            {:connection_mode => :per_thread}
+            { :connection_mode => :per_thread }
           end
         end
       end
-- 
1.7.4.4


Re: [PATCH 2/2] EC2: do not modify env variables EC2_URL and S3_URL

Posted by Jason Guiditta <ja...@gmail.com>.
I've updated my local copy with these changes and can now connect to ec2 w/o
issue, thanks for the quick fix!

-j

On Tue, Apr 19, 2011 at 2:56 PM, David Lutterkort <lu...@redhat.com> wrote:

> On Mon, 2011-04-18 at 21:51 -0700, Sang-Min Park wrote:
> > Actually, I'd suggest:
> > ENV['EC2_URL'] = nil
> > ENV['S3_URL'] = nil.
> >
> > AWS overrides :endpoint_url if the environmental variables are set.
> > The typical eucalyptus installation will have the two env. variables set
> to
> > Eucalyptus/Walrus backend.
> > If a Eucalyptus user starts deltacloud process and if she switches to EC2
> > driver, AWS will attempt to connect Eucalyptus backend.
> >
> > If we set them nil, AWS will use the default endpoint for EC2 or the one
> set
> > through EC2 provider. I tested and confirmed it works.
>
> Good point - I've modified the driver to delete EC2_URL, S3_URL and
> ELB_URL from the environment.
>
> Users should use Deltacloud mechanisms to select endpoints; that we use
> AWS is an implementation detail, anyway.
>
> David
>
>
>

Re: [PATCH 2/2] EC2: do not modify env variables EC2_URL and S3_URL

Posted by David Lutterkort <lu...@redhat.com>.
On Mon, 2011-04-18 at 21:51 -0700, Sang-Min Park wrote:
> Actually, I'd suggest:
> ENV['EC2_URL'] = nil
> ENV['S3_URL'] = nil.
> 
> AWS overrides :endpoint_url if the environmental variables are set.
> The typical eucalyptus installation will have the two env. variables set to
> Eucalyptus/Walrus backend.
> If a Eucalyptus user starts deltacloud process and if she switches to EC2
> driver, AWS will attempt to connect Eucalyptus backend.
> 
> If we set them nil, AWS will use the default endpoint for EC2 or the one set
> through EC2 provider. I tested and confirmed it works.

Good point - I've modified the driver to delete EC2_URL, S3_URL and
ELB_URL from the environment.

Users should use Deltacloud mechanisms to select endpoints; that we use
AWS is an implementation detail, anyway.

David



Re: [PATCH 2/2] EC2: do not modify env variables EC2_URL and S3_URL

Posted by Sang-Min Park <sa...@eucalyptus.com>.
Actually, I'd suggest:
ENV['EC2_URL'] = nil
ENV['S3_URL'] = nil.

AWS overrides :endpoint_url if the environmental variables are set.
The typical eucalyptus installation will have the two env. variables set to
Eucalyptus/Walrus backend.
If a Eucalyptus user starts deltacloud process and if she switches to EC2
driver, AWS will attempt to connect Eucalyptus backend.

If we set them nil, AWS will use the default endpoint for EC2 or the one set
through EC2 provider. I tested and confirmed it works.

Sang-min


On Mon, Apr 18, 2011 at 3:35 PM, <lu...@redhat.com> wrote:

> From: David Lutterkort <lu...@redhat.com>
>
> Setting these variables to the empty string caused the aws gem to issue
> invalid signatures on requests.
>
> This issue was introduced in commit b8e725c6
> ---
>  server/lib/deltacloud/drivers/ec2/ec2_driver.rb |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> index dd67422..83054c2 100644
> --- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> +++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> @@ -537,8 +537,6 @@ module Deltacloud
>         end
>
>         def endpoint_for_service(service)
> -          ENV['EC2_URL']=''  # unset endpoints that may have been set by
> eucalyptus; otherwise it can conflict with the EC2/S3 endpoints in aws gem.
> -          ENV['S3_URL']=''
>           endpoint = (Thread.current[:provider] || ENV['API_PROVIDER'] ||
> DEFAULT_REGION)
>           # return the endpoint if it does not map to a default endpoint,
> allowing
>           # the endpoint to be a full hostname instead of a region.
> --
> 1.7.4.4
>
>

[PATCH 2/2] EC2: do not modify env variables EC2_URL and S3_URL

Posted by lu...@redhat.com.
From: David Lutterkort <lu...@redhat.com>

Setting these variables to the empty string caused the aws gem to issue
invalid signatures on requests.

This issue was introduced in commit b8e725c6
---
 server/lib/deltacloud/drivers/ec2/ec2_driver.rb |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
index dd67422..83054c2 100644
--- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
+++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
@@ -537,8 +537,6 @@ module Deltacloud
         end
 
         def endpoint_for_service(service)
-          ENV['EC2_URL']=''  # unset endpoints that may have been set by eucalyptus; otherwise it can conflict with the EC2/S3 endpoints in aws gem.
-          ENV['S3_URL']=''
           endpoint = (Thread.current[:provider] || ENV['API_PROVIDER'] || DEFAULT_REGION)
           # return the endpoint if it does not map to a default endpoint, allowing
           # the endpoint to be a full hostname instead of a region.
-- 
1.7.4.4