[PATCH conductor] Verify image exists before importing it from provider

Matt Wagner matt.wagner at redhat.com
Fri Dec 16 21:06:28 UTC 2011


Resolves https://bugzilla.redhat.com/show_bug.cgi?id=765714
---
 src/app/controllers/api/images_controller.rb       |    7 +++-
 src/app/controllers/images_controller.rb           |    6 ++-
 src/config/locales/en.yml                          |    2 +
 src/lib/exceptions.rb                              |    4 ++
 src/lib/image.rb                                   |    8 +++-
 src/spec/controllers/api/images_controller_spec.rb |   41 +++++++++++++++++++-
 6 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/src/app/controllers/api/images_controller.rb b/src/app/controllers/api/images_controller.rb
index db82553..3d3c448 100644
--- a/src/app/controllers/api/images_controller.rb
+++ b/src/app/controllers/api/images_controller.rb
@@ -68,7 +68,12 @@ module Api
           account = ProviderAccount.find_by_label(req[:params][:provider_account_name])
           raise(Aeolus::Conductor::API::ProviderAccountNotFound.new(404, t("api.error_messages.provider_account_not_found",
             :name => req[:params][:provider_account_name]))) unless account.present?
-          @image = Image.import(account, req[:params][:target_identifier], req[:params][:image_descriptor])
+          begin
+            @image = Image.import(account, req[:params][:target_identifier], req[:params][:image_descriptor])
+          rescue Aeolus::Conductor::UI::ImageNotFound
+            raise(Aeolus::Conductor::API::ImageNotFound.new(404, t("api.error_messages.image_not_found_on_provider",
+              :image => req[:params][:target_identifier])))
+          end
           respond_with(@image)
         end
       rescue ActiveResource::BadRequest => e
diff --git a/src/app/controllers/images_controller.rb b/src/app/controllers/images_controller.rb
index 2adce72..a3d1e20 100644
--- a/src/app/controllers/images_controller.rb
+++ b/src/app/controllers/images_controller.rb
@@ -97,7 +97,11 @@ class ImagesController < ApplicationController
       redirect_to image_url(image.id) and return
     rescue Exception => e
       logger.error "Caught exception importing image: #{e.message}"
-      flash[:error] = t("images.import.image_not_imported")
+      if e.is_a?(Aeolus::Conductor::UI::ImageNotFound)
+        flash[:error] = t('images.not_on_provider')
+      else
+        flash[:error] = t("images.import.image_not_imported")
+      end
       redirect_to new_image_url(:tab => 'import')
     end
   end
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index 9f4742f..d478e03 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -738,6 +738,7 @@ en:
       make_deployable: Automatically make "%{name}" deployable.
       back: Back
       save_template: Save Template
+    not_on_provider: The requested image was not found on the provider.
   template_xml:
     errors:
       xml_parse_error: Failed to parse XML.
@@ -1180,6 +1181,7 @@ en:
     error_messages:
       build_not_found: Could not find Build %{build}
       image_not_found: Could not find Image %{image}
+      image_not_found_on_provider: "Could not find Image %{image} on provider"
       in_grabbing_target_images: "In grabbing list of target images: %{error}"
       invalid_parameters_for_account_and_target_image: "Invalid parameters for Account: %{account} TargetImage: %{targetimage}"
       no_image_build_or_target_image: "Invalid Parameters: No Image, Build or TargetImage provided in request"
diff --git a/src/lib/exceptions.rb b/src/lib/exceptions.rb
index 2068fe6..6c11681 100644
--- a/src/lib/exceptions.rb
+++ b/src/lib/exceptions.rb
@@ -42,5 +42,9 @@ module Aeolus
       class TargetImageStatusNotFound < Error; end
       class TargetNotFound < Error; end
     end
+
+    module UI
+      class ImageNotFound < StandardError; end
+    end
   end
 end
diff --git a/src/lib/image.rb b/src/lib/image.rb
index 2c8db1a..d903e94 100644
--- a/src/lib/image.rb
+++ b/src/lib/image.rb
@@ -16,8 +16,14 @@
 
 class Image
   # Given a ProviderAccount and an AMI/image ID on a provider, plus an optional XML string, use aeolus-image
-  # to import the image. Returns an Aeolus::Image::Factory::Image or returns any raised exceptions
+  # to import the image. Returns an Aeolus::Image::Factory::Image or allows exceptions to bubble up
   def self.import(provider_account, image_id, xml=nil)
+    # Verify that the image exists prior to import
+    conn = provider_account.connect
+    img = conn.image(image_id) rescue nil
+    raise Aeolus::Conductor::UI::ImageNotFound unless img.present?
+    # We have the image name in the cloud provider, so we might as well use it
+    xml ||= "<image><name>#{img.name}</name></image>" if img.name.present?
     provider = provider_account.provider
     account_id = provider_account.credentials_hash['username']
     Aeolus::Image.import(provider.name, provider.provider_type.deltacloud_driver, image_id, account_id, xml)
diff --git a/src/spec/controllers/api/images_controller_spec.rb b/src/spec/controllers/api/images_controller_spec.rb
index 09481c8..d3fb24f 100644
--- a/src/spec/controllers/api/images_controller_spec.rb
+++ b/src/spec/controllers/api/images_controller_spec.rb
@@ -257,11 +257,15 @@ describe Api::ImagesController do
             # We previously stubbed this out to return nil... That's inappropriate here:
             Aeolus::Image::Warehouse::Image.stub(:find).and_return(@image)
             @provider_image.stub(:set_attr)
+            @deltacloud_connection = mock("Deltacloud::API")
+            @provider_account.stub(:connect).and_return(@deltacloud_connection)
+            @deltacloud_image = mock("Deltacloud::API::Stateful::Image",
+              :name => 'mock-image')
+            @deltacloud_connection.stub(:image).and_return(@deltacloud_image)
 
             request.env['RAW_POST_DATA'] = xml.to_xml
             post :create
           end
-
           it { response.response_code == 200 }
           it { response.headers['Content-Type'].should include("application/xml") }
           it "should have an image with correct attributes" do
@@ -273,6 +277,41 @@ describe Api::ImagesController do
 
         end
 
+        context "when trying to import image that does not exist" do
+          before(:each) do
+            xml = Nokogiri::XML::Builder.new do
+              image {
+                target_identifier "tid2"
+                image_descriptor {
+                  child "c3"
+                  child "c4"
+                }
+                provider_account_name "mock2"
+              }
+            end
+            Aeolus::Image::Factory::Image.stub(:new).and_return(@image)
+            ProviderAccount.stub(:find_by_label).and_return(@provider_account)
+            @provider_account.stub(:provider).and_return(@provider)
+            @image.stub(:save!)
+            # We previously stubbed this out to return nil... That's inappropriate here:
+            Aeolus::Image::Warehouse::Image.stub(:find).and_return(@image)
+            @provider_image.stub(:set_attr)
+            @deltacloud_connection = mock("Deltacloud::API")
+            @provider_account.stub(:connect).and_return(@deltacloud_connection)
+            @deltacloud_connection.stub(:image).and_raise("no image")
+
+            request.env['RAW_POST_DATA'] = xml.to_xml
+            post :create
+          end
+          it { response.response_code == 404 }
+          it { response.headers['Content-Type'].should include("application/xml") }
+          it "should include an error" do
+            resp = Hash.from_xml(response.body)
+            resp['error']['message'].should == "Could not find Image tid2 on provider"
+         end
+
+        end
+
       end
 
       describe "#destroy" do
-- 
1.7.6.4




More information about the aeolus-devel mailing list