stager: add upload and download to the droplet and buildpack_cache ur…

…is so that nginx doesn't swallow download requests from deas
youngm · May 9, 2013 · ee05314 · ee05314
1 parent d53255d
commit ee05314
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 30 deletions.
diff --git a/config/nginx.conf b/config/nginx.conf
@@ -80,8 +80,7 @@ http {
       upload_cleanup 400-505;
     }
 
-    # Droplet uploads from the stager should be authenticated
-    location ~ /staging/(buildpack_cache|droplets)/ {
+    location ~ /staging/(buildpack_cache|droplets)/.*/upload/ {
       # Pass along auth header
       set $auth_header $upstream_http_x_auth;
       proxy_set_header Authorization $auth_header;

diff --git a/lib/cloud_controller/api/staging.rb b/lib/cloud_controller/api/staging.rb
@@ -60,7 +60,7 @@ def buildpack_cache_upload_uri(guid)
 
       def droplet_download_uri(guid)
         if local?
-          staging_uri("#{DROPLET_PATH}/#{guid}")
+          staging_uri("#{DROPLET_PATH}/#{guid}/download")
         else
           droplet_uri(guid)
         end
@@ -116,7 +116,7 @@ def local?
 
       def buildpack_cache_download_uri(guid)
         if AppPackage.local?
-          staging_uri("#{BUILDPACK_CACHE_PATH}/#{guid}")
+          staging_uri("#{BUILDPACK_CACHE_PATH}/#{guid}/download")
         else
           package_uri(guid, :buildpack_cache)
         end
@@ -155,11 +155,8 @@ def store_package(guid, path, type)
       end
 
       def upload_uri(guid, type)
-        if type == :buildpack_cache
-          staging_uri("#{BUILDPACK_CACHE_PATH}/#{guid}")
-        else
-          staging_uri("#{DROPLET_PATH}/#{guid}")
-        end
+        prefix = type == :buildpack_cache ? BUILDPACK_CACHE_PATH : DROPLET_PATH
+        staging_uri("#{prefix}/#{guid}/upload")
       end
 
       def package_uri(guid, type)
@@ -371,10 +368,12 @@ def tmpdir
     end
 
     get  "/staging/apps/:guid", :download_app
-    post "#{DROPLET_PATH}/:guid", :upload_droplet
-    get  "#{DROPLET_PATH}/:guid", :download_droplet
 
-    post "#{BUILDPACK_CACHE_PATH}/:guid", :upload_buildpack_cache
-    get "#{BUILDPACK_CACHE_PATH}/:guid", :download_buildpack_cache
+    # Make sure that nginx upload path rules do not apply to download paths!
+    post "#{DROPLET_PATH}/:guid/upload", :upload_droplet
+    get  "#{DROPLET_PATH}/:guid/download", :download_droplet
+
+    post "#{BUILDPACK_CACHE_PATH}/:guid/upload", :upload_buildpack_cache
+    get "#{BUILDPACK_CACHE_PATH}/:guid/download", :download_buildpack_cache
   end
 end
diff --git a/spec/api/staging_spec.rb b/spec/api/staging_spec.rb
@@ -149,27 +149,36 @@ def self.it_destroys_handle
     describe "droplet_upload_uri" do
       it "should return a uri to our cc" do
         uri = Staging.droplet_upload_uri(app_guid)
-        uri.should == "http://#{staging_user}:#{staging_password}@#{cc_addr}:#{cc_port}/staging/droplets/#{app_guid}"
+        uri.should == "http://#{staging_user}:#{staging_password}@#{cc_addr}:#{cc_port}/staging/droplets/#{app_guid}/upload"
+      end
+    end
+
+    describe "droplet_download_uri" do
+      let(:droplet) { Tempfile.new(app_guid) }
+      before { Staging.store_droplet(app_guid, droplet.path) }
+      after { FileUtils.rm(droplet.path) }
+
+      it "returns internal cc uri" do
+        uri = Staging.droplet_download_uri(app_guid)
+        uri.should == "http://#{staging_user}:#{staging_password}@#{cc_addr}:#{cc_port}/staging/droplets/#{app_guid}/download"
       end
     end
 
     describe "buildpack_cache_upload_uri" do
       it "should return a uri to our cc" do
         uri = Staging.buildpack_cache_upload_uri(app_guid)
-        uri.should == "http://#{staging_user}:#{staging_password}@#{cc_addr}:#{cc_port}/staging/buildpack_cache/#{app_guid}"
+        uri.should == "http://#{staging_user}:#{staging_password}@#{cc_addr}:#{cc_port}/staging/buildpack_cache/#{app_guid}/upload"
       end
     end
 
     describe "buildpack_cache_download_uri" do
       let(:buildpack_cache) { Tempfile.new(app_guid) }
-
+      before { Staging.store_buildpack_cache(app_guid, buildpack_cache.path) }
       after { FileUtils.rm(buildpack_cache.path) }
 
-      it "should return a uri to our cc" do
-        Staging.store_buildpack_cache(app_guid, buildpack_cache.path)
-
+      it "returns internal cc uri" do
         uri = Staging.buildpack_cache_download_uri(app_guid)
-        uri.should == "http://#{staging_user}:#{staging_password}@#{cc_addr}:#{cc_port}/staging/buildpack_cache/#{app_guid}"
+        uri.should == "http://#{staging_user}:#{staging_password}@#{cc_addr}:#{cc_port}/staging/buildpack_cache/#{app_guid}/download"
       end
     end
 
@@ -231,7 +240,7 @@ def self.it_downloads_staged_app
       end
     end
 
-    describe "POST /staging/droplets/:guid" do
+    describe "POST /staging/droplets/:guid/upload" do
       let(:app_obj) { Models::App.make }
       let(:tmpfile) { Tempfile.new("droplet.tgz") }
       let(:upload_req) do
@@ -244,7 +253,7 @@ def self.it_downloads_staged_app
       end
 
       def make_request(droplet_guid=app_obj.guid)
-        post "/staging/droplets/#{droplet_guid}", upload_req
+        post "/staging/droplets/#{droplet_guid}/upload", upload_req
       end
 
       context "with a valid upload handle" do
@@ -281,7 +290,7 @@ def make_request(droplet_guid=app_obj.guid)
       include_examples "staging bad auth", :post
     end
 
-    describe "GET /staging/droplets/:guid" do
+    describe "GET /staging/droplets/:guid/download" do
       let(:app_obj) { Models::App.make }
 
       before do
@@ -307,28 +316,28 @@ def make_request(droplet_guid=app_obj.guid)
           droplet.close
           Staging.store_droplet(app_obj.guid, droplet.path)
 
-          get "/staging/droplets/#{app_obj.guid}"
+          get "/staging/droplets/#{app_obj.guid}/download"
           last_response.status.should == 200
           last_response.headers["X-Accel-Redirect"].should match("/cc-droplets/.*/#{app_obj.guid}")
         end
       end
 
       context "with a valid app but no droplet" do
         it "should return an error" do
-          get "/staging/droplets/#{app_obj.guid}"
+          get "/staging/droplets/#{app_obj.guid}/download"
           last_response.status.should == 400
         end
       end
 
       context "with an invalid app" do
         it "should return an error" do
-          get "/staging/droplets/bad"
+          get "/staging/droplets/bad/download"
           last_response.status.should == 404
         end
       end
     end
 
-    describe "POST /staging/buildpack_cache/:guid" do
+    describe "POST /staging/buildpack_cache/:guid/upload" do
       let(:app_obj) { Models::App.make }
       let(:tmpfile) { Tempfile.new("droplet.tgz") }
       let(:upload_req) do
@@ -341,7 +350,7 @@ def make_request(droplet_guid=app_obj.guid)
       end
 
       def make_request(droplet_guid=app_obj.guid)
-        post "/staging/buildpack_cache/#{droplet_guid}", upload_req
+        post "/staging/buildpack_cache/#{droplet_guid}/upload", upload_req
       end
 
       context "with a valid buildpack cache upload handle" do
@@ -376,7 +385,7 @@ def make_request(droplet_guid=app_obj.guid)
       end
     end
 
-    describe "GET /staging/buildpack_cache/:guid" do
+    describe "GET /staging/buildpack_cache/:guid/download" do
       let(:app_obj) { Models::App.make }
       let(:buildpack_cache) { Tempfile.new(app_obj.guid) }
 
@@ -390,7 +399,7 @@ def make_request(droplet_guid=app_obj.guid)
       after { FileUtils.rm(buildpack_cache.path) }
 
       def make_request(droplet_guid=app_obj.guid)
-        get "/staging/buildpack_cache/#{droplet_guid}"
+        get "/staging/buildpack_cache/#{droplet_guid}/download"
       end
 
       context "with a valid buildpack cache" do