[PATCH aeolus-cli] BZ#747719 error message for image import with incorrect parms

[Maros Zatko]

https://bugzilla.redhat.com/show_bug.cgi?id=747719

This patchset is without test.
I'm sending it just in case we don't have tests ready today.
If we do, there will be other patchset with tests included, sent by Petr.

Sorry for any inconvenience,
Maros

[PATCH 1/6] bz#747719: validation of import parameters

[Maros Zatko]

From: Maros Zatko

---
 lib/aeolus_cli/command/import_command.rb |   50 +++++++++++++++++++++++++++---
 1 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/lib/aeolus_cli/command/import_command.rb b/lib/aeolus_cli/command/import_command.rb
index 591a999..91a5d54 100644
--- a/lib/aeolus_cli/command/import_command.rb
+++ b/lib/aeolus_cli/command/import_command.rb
@@ -18,11 +18,7 @@ module Aeolus
       def initialize(opts={}, logger=nil)
         super(opts, logger)
         default = {
-          :image => '',
-          :build => '', # TODO - Is this used anywhere?
-          :id => '',
           :description => '<image><name>' + @options[:id] + '</name></image>',
-          :provider_account => ''
         }
         @options = default.merge(@options)
       end
@@ -33,7 +29,9 @@ module Aeolus
           if !description.nil?
             @options[:description] = description
           end
-          # TODO: Validate Description XML
+
+          import_params_valid!(@options)
+
           image = Aeolus::CLI::Image.new({:target_identifier => @options[:id],
                                           :image_descriptor => @options[:description],
                                           :provider_account_name => @options[:provider_account].first})
@@ -49,6 +47,48 @@ module Aeolus
           handle_exception(e)
         end
       end
+
+      private
+
+      def import_params_valid!(params)
+        if not params.is_a? Hash
+          raise TypeError, "params should be Hash instead of #{params.class}"
+        end
+
+        if not params.values.all?
+          raise ArgumentError, "params should not contain nil"
+        end
+
+        required_keys = [:id, :target, :description, :provider]
+
+        isect = params.keys & required_keys
+        diff  = params.keys - required_keys
+
+        missing = required_keys - isect
+
+        if not missing == []
+          raise ArgumentError, "missing #{missing*', '}"
+        end
+
+        if not diff == []
+          raise ArgumentError, "unexpected #{diff*', '}"
+        end
+
+        validate_description_xml!(params[:description])
+      end
+
+      def validate_description_xml!(xml)
+        errors = validate_xml_document(File.dirname(__FILE__) + "/../../../examples/image_desc.rng", xml)
+        if errors.length > 0
+          puts "ERROR: The given image descrition does not conform to the xml schema, see below for specific details:"
+          errors.each do |error|
+            puts "- " + error.message
+          end
+          raise ArgumentError, "Invalid description"
+        end
+        true
+      end
+
     end
   end
 end

[PATCH 1/6] bz#747719: validation of import parameters

[Hugh O. Brock]

On Thu, Dec 22, 2011 at 06:42:53PM +0100, [e-mail] wrote:
> From: Maros Zatko
>
> lib/aeolus_cli/command/import_command.rb |   50 +++++++++++++++++++++++++++---
>  1 files changed, 45 insertions(+), 5 deletions(-)
>
> diff --git a/lib/aeolus_cli/command/import_command.rb b/lib/aeolus_cli/command/import_command.rb
> index 591a999..91a5d54 100644
> --- a/lib/aeolus_cli/command/import_command.rb
> +++ b/lib/aeolus_cli/command/import_command.rb
> @@ -18,11 +18,7 @@ module Aeolus
>        def initialize(opts={}, logger=nil)
>          super(opts, logger)
>          default = {
>      :image => '',
>      :build => '', # TODO - Is this used anywhere?
>      :id => '',
>      :description => '<image><name>' + @options[:id] + '</name></image>',
>      :provider_account => ''
>    }
>    @options = default.merge(@options)
>  end
>
> @@ -33,7 +29,9 @@ module Aeolus
>            if !description.nil?
>              @options[:description] = description
>            end
>      # TODO: Validate Description XML
>      import_params_valid!(@options)
>      image = Aeolus::CLI::Image.new({:target_identifier => @options[:id],
>                                      :image_descriptor => @options[:description],
>                                      :provider_account_name => @options[:provider_account].first})
>
> @@ -49,6 +47,48 @@ module Aeolus
>            handle_exception(e)
>          end
>        end
>  private
>  def import_params_valid!(params)
>    if not params.is_a? Hash
>      raise TypeError, "params should be Hash instead of #{params.class}"
>    end
>    if not params.values.all?
>      raise ArgumentError, "params should not contain nil"
>    end
>    required_keys = [:id, :target, :description, :provider]
>    isect = params.keys & required_keys
>    diff  = params.keys - required_keys
>    missing = required_keys - isect
>    if not missing == []
>      raise ArgumentError, "missing #{missing*', '}"
>    end
>    if not diff == []
>      raise ArgumentError, "unexpected #{diff*', '}"
>    end
>    validate_description_xml!(params[:description])
>  end
>  def validate_description_xml!(xml)
>    errors = validate_xml_document(File.dirname(__FILE__) + "/../../../examples/image_desc.rng", xml)
>    if errors.length > 0
>      puts "ERROR: The given image descrition does not conform to the xml schema, see below for specific details:"

ITYM "description"                        ^^^^^^^^^^
>      errors.each do |error|
>        puts "- " + error.message
>      end
>      raise ArgumentError, "Invalid description"
>    end
>    true
>  end
> end
>  end
> end
>
> 1.7.7.4

(Just happened to see that...)

--H

[PATCH 2/6] bz#747719: error handling for import params validations

[Maros Zatko]

From: Maros Zatko

---
 lib/aeolus_cli/command/base_command.rb |   74 +++++++++++++++++++++++---------
 1 files changed, 54 insertions(+), 20 deletions(-)

diff --git a/lib/aeolus_cli/command/base_command.rb b/lib/aeolus_cli/command/base_command.rb
index 16573c5..47a9d2d 100644
--- a/lib/aeolus_cli/command/base_command.rb
+++ b/lib/aeolus_cli/command/base_command.rb
@@ -86,33 +86,67 @@ module Aeolus

private
       def handle_exception(e)
-        doc = Nokogiri::XML e.response.body
-        code = doc.xpath("/error/code").text
-        message = doc.xpath("/error/message").text

if e.is_a?(ActiveResource::UnauthorizedAccess)
           code = "Unauthorized"
           message = "Invalid Credentials, please check ~/.aeolus-cli"
-        elsif message.to_s.empty?
+
+        elsif e.is_a?(TypeError)
+          code = "Internal Error (TypeError)"
+          message = e.message
+
+        elsif e.is_a?(ArgumentError)
+          code = "Argument Error"
+          message = e.message
+
+        elsif e.is_a?(ActiveResource::ResourceNotFound)
+          code = "ResourceNotFound"
+          message = "Could not find resource"
+
+        elsif e.respond_to?(:response)
+          doc = Nokogiri::XML e.response.body
+          code = doc.xpath("/error/code").text
+          message = doc.xpath("/error/message").text
+
           case code
-            when "BuildDeleteFailure" : message = "An error occured when deleting the Build from the Image Warehouse"
-            when "BuildNotFound" : message = "Could not find the specified Build"
-            when "ImageDeleteFailure" : message = "An error occured when deleting the Image from the Image Warehouse"
-            when "ImageNotFound" : message = "Could not find the specified Image"
-            when "InsufficientParametersSupplied" : message = "There were insufficient parameters provided in the request"
-            when "ParameterDataIncorrect" : message = "The given parameters are incorrect"
-            when "PushError" : message = "An error occured the Image Factory when trying to push"
-            when "ProviderAccountNotFound" : message = "Could not find the specified account"
-            when "ProviderImageDeleteFailure" : message = "An error occurd when deleting the Provider Image from Image Warehouse"
-            when "ProviderImageNotFound" : message = "Could not find the specified Provider Image"
-            when "ProviderImageStatusNotFound" : message = "There was no status supplied in the Provider Image"
-            when "TargetImageDeleteFailure" : message = "An error occured when deleting the Target Image from the Image Warehouse"
-            when "TargetImageNotFound" : message = "Could not locate the specified Target Image"
-            when "TargetImageStatusNotFound" : message = "There was no status supplied in the Target Image"
-            when "TargetNotFound" : message = "Could not locate the specified Target"
-            else message = "Unknown Error"
+            when "BuildDeleteFailure"
+              message = "An error occured when deleting the Build from the Image Warehouse"
+            when "BuildNotFound"
+              message = "Could not find the specified Build"
+            when "ImageDeleteFailure"
+              message = "An error occured when deleting the Image from the Image Warehouse"
+            when "ImageNotFound"
+              message = "Could not find the specified Image"
+            when "InsufficientParametersSupplied"
+              message = "There were insufficient parameters provided in the request"
+            when "ParameterDataIncorrect"
+              message = "The given parameters are incorrect"
+            when "PushError"
+              message = "An error occured the Image Factory when trying to push"
+            when "ProviderAccountNotFound"
+              message = "Could not find the specified account"
+            when "ProviderImageDeleteFailure"
+              message = "An error occurd when deleting the Provider Image from Image Warehouse"
+            when "ProviderImageNotFound"
+              message = "Could not find the specified Provider Image"
+            when "ProviderImageStatusNotFound"
+              message = "There was no status supplied in the Provider Image"
+            when "TargetImageDeleteFailure"
+              message = "An error occured when deleting the Target Image from the Image Warehouse"
+            when "TargetImageNotFound"
+              message = "Could not locate the specified Target Image"
+            when "TargetImageStatusNotFound"
+              message = "There was no status supplied in the Target Image"
+            when "TargetNotFound"
+              message = "Could not locate the specified Target"
+            else
+              message = e.message
           end
+        else
+          message = e.message + "\n" + (e.backtrace * "\n")
+          code = 'Unknown Error'
         end
+
         puts ""
         puts "ERROR:  " + code + " => " + message
         quit(1)

[PATCH 3/6] bz#747719: Relax NG schema for image description xml

[Maros Zatko]

From: Maros Zatko

---
 examples/image_desc.rng |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)
 create mode 100644 examples/image_desc.rng

diff --git a/examples/image_desc.rng b/examples/image_desc.rng
new file mode 100644
index 0000000..16fd41f
--- /dev/null
+++ b/examples/image_desc.rng
@@ -0,0 +1,16 @@
+<?xml version="1.0"?>
+<!-- A Relax NG schema for image description xml -->
+<grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes">
+  <start>
+    <ref name='image'/>
+  </start>
+
+  <define name='image'>
+    <element name='image'>
+      <element name='name'>
+        <text/>
+      </element>
+    </element>
+  </define>
+
+</grammar>

[PATCH 4/6] fix known error code condition

[Maros Zatko]

From: Maros Zatko

---
 lib/aeolus_cli/command/base_command.rb |   74 ++++++++++++++++---------------
 1 files changed, 38 insertions(+), 36 deletions(-)

diff --git a/lib/aeolus_cli/command/base_command.rb b/lib/aeolus_cli/command/base_command.rb
index 47a9d2d..91ef445 100644
--- a/lib/aeolus_cli/command/base_command.rb
+++ b/lib/aeolus_cli/command/base_command.rb
@@ -99,48 +99,50 @@ module Aeolus
           code = "Argument Error"
           message = e.message

-        elsif e.is_a?(ActiveResource::ResourceNotFound)
-          code = "ResourceNotFound"
-          message = "Could not find resource"
+#        elsif e.is_a?(ActiveResource::ResourceNotFound)
+#          code = "ResourceNotFound"
+#          message = "Could not find resource"

elsif e.respond_to?(:response)
           doc = Nokogiri::XML e.response.body
           code = doc.xpath("/error/code").text
           message = doc.xpath("/error/message").text

-          case code
-            when "BuildDeleteFailure"
-              message = "An error occured when deleting the Build from the Image Warehouse"
-            when "BuildNotFound"
-              message = "Could not find the specified Build"
-            when "ImageDeleteFailure"
-              message = "An error occured when deleting the Image from the Image Warehouse"
-            when "ImageNotFound"
-              message = "Could not find the specified Image"
-            when "InsufficientParametersSupplied"
-              message = "There were insufficient parameters provided in the request"
-            when "ParameterDataIncorrect"
-              message = "The given parameters are incorrect"
-            when "PushError"
-              message = "An error occured the Image Factory when trying to push"
-            when "ProviderAccountNotFound"
-              message = "Could not find the specified account"
-            when "ProviderImageDeleteFailure"
-              message = "An error occurd when deleting the Provider Image from Image Warehouse"
-            when "ProviderImageNotFound"
-              message = "Could not find the specified Provider Image"
-            when "ProviderImageStatusNotFound"
-              message = "There was no status supplied in the Provider Image"
-            when "TargetImageDeleteFailure"
-              message = "An error occured when deleting the Target Image from the Image Warehouse"
-            when "TargetImageNotFound"
-              message = "Could not locate the specified Target Image"
-            when "TargetImageStatusNotFound"
-              message = "There was no status supplied in the Target Image"
-            when "TargetNotFound"
-              message = "Could not locate the specified Target"
-            else
-              message = e.message
+          if message.to_s.empty?
+            case code
+              when "BuildDeleteFailure"
+                message = "An error occured when deleting the Build from the Image Warehouse"
+              when "BuildNotFound"
+                message = "Could not find the specified Build"
+              when "ImageDeleteFailure"
+                message = "An error occured when deleting the Image from the Image Warehouse"
+              when "ImageNotFound"
+                message = "Could not find the specified Image"
+              when "InsufficientParametersSupplied"
+                message = "There were insufficient parameters provided in the request"
+              when "ParameterDataIncorrect"
+                message = "The given parameters are incorrect"
+              when "PushError"
+                message = "An error occured the Image Factory when trying to push"
+              when "ProviderAccountNotFound"
+                message = "Could not find the specified account"
+              when "ProviderImageDeleteFailure"
+                message = "An error occurd when deleting the Provider Image from Image Warehouse"
+              when "ProviderImageNotFound"
+                message = "Could not find the specified Provider Image"
+              when "ProviderImageStatusNotFound"
+                message = "There was no status supplied in the Provider Image"
+              when "TargetImageDeleteFailure"
+                message = "An error occured when deleting the Target Image from the Image Warehouse"
+              when "TargetImageNotFound"
+                message = "Could not locate the specified Target Image"
+              when "TargetImageStatusNotFound"
+                message = "There was no status supplied in the Target Image"
+              when "TargetNotFound"
+                message = "Could not locate the specified Target"
+              else
+                message = e.message
+            end
           end
         else
           message = e.message + "\n" + (e.backtrace * "\n")

[PATCH 5/6] change :provider to :provider_account

[Maros Zatko]

From: Maros Zatko

---
 lib/aeolus_cli/command/import_command.rb |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/aeolus_cli/command/import_command.rb b/lib/aeolus_cli/command/import_command.rb
index 91a5d54..c765d4e 100644
--- a/lib/aeolus_cli/command/import_command.rb
+++ b/lib/aeolus_cli/command/import_command.rb
@@ -59,7 +59,7 @@ module Aeolus
           raise ArgumentError, "params should not contain nil"
         end

-        required_keys = [:id, :target, :description, :provider]
+        required_keys = [:id, :target, :description, :provider_account]

isect = params.keys & required_keys
         diff  = params.keys - required_keys

[PATCH 6/6] change :provider to :provider_account

[Maros Zatko]

From: Maros Zatko

---
 spec/command/import_command_spec.rb |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/spec/command/import_command_spec.rb b/spec/command/import_command_spec.rb
index 29b9ce8..bf3e922 100644
--- a/spec/command/import_command_spec.rb
+++ b/spec/command/import_command_spec.rb
@@ -20,7 +20,7 @@ module Aeolus
       before(:each) do
         @options = { :id => "ami-5592553c",
                      :target => ["ec2"],
-                     :provider => ["ec2-us-east-1"],
+                     :provider_account => ["ec2-us-east-1"],
                      :description => "<image><name>MyImage</name></image>"}
       end