From: Maros Zatko <mzatko(a)redhat.com&gt; --- 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 -- 1.7.7.4

From: Maros Zatko <mzatko(a)redhat.com&gt; --- 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> -- 1.7.7.4

From: Maros Zatko <mzatko(a)redhat.com&gt; --- 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 -- 1.7.7.4

--- spec/command/base_command_spec.rb | 30 ++++++++++++------------------ 1 files changed, 12 insertions(+), 18 deletions(-) diff --git a/spec/command/base_command_spec.rb b/spec/command/base_command_spec.rb index bc8a0c1..26b044b 100644 --- a/spec/command/base_command_spec.rb +++ b/spec/command/base_command_spec.rb @@ -17,9 +17,11 @@ require 'spec_helper' module Aeolus module CLI describe BaseCommand do + let( :base_command ) { BaseCommand.new } + describe "configure_active_resource" do it "should setup ActiveResource with endpoint and authentication" do - basec = BaseCommand.new + base_command Aeolus::CLI::Base.site.to_s.should == "https://localhost/conductor/api" Aeolus::CLI::Base.user.to_s.should == "admin" Aeolus::CLI::Base.password.to_s.should == "password" @@ -27,38 +29,32 @@ module Aeolus end describe "#read_file" do - it "should return nil when it cannot find file" do - b = BaseCommand.new - b.send(:read_file, "foo.fake").should be_nil + base_command.send(:read_file, "foo.fake").should be_nil end it "should read file content into string variable" do - b = BaseCommand.new - template_str = b.send(:read_file, "#{File.dirname(__FILE__)}" + "/../../examples/custom_repo.tdl") + template_str = base_command.send(:read_file, "#{File.dirname(__FILE__)}" + "/../../examples/custom_repo.tdl") template_str.should include("<template>") end end describe "#is_file?" do it "should return false if no file found" do - b = BaseCommand.new - b.send(:is_file?, "foo.fake").should be_false + base_command.send(:is_file?, "foo.fake").should be_false end it "should return true if file found" do - b = BaseCommand.new valid_file = "#{File.dirname(__FILE__)}" + "/../../examples/aeolus-cli" - b.instance_eval {@config_location = valid_file} - b.send(:is_file?, valid_file).should be_true + base_command.instance_eval {@config_location = valid_file} + base_command.send(:is_file?, valid_file).should be_true end end describe "#write_file" do it "should write a new file" do - b = BaseCommand.new new_file = "/tmp/foo.fake" - b.instance_eval {@config_location = new_file} - b.send(:write_file) + base_command.instance_eval {@config_location = new_file} + base_command.send(:write_file) conf = YAML::load(File.open(File.expand_path(new_file))) conf.has_key?(:conductor).should be_true File.delete(new_file) @@ -67,14 +63,12 @@ module Aeolus describe "#validate_xml_document?" do it "should return errors given non compliant xml" do - b = BaseCommand.new - errors = b.send(:validate_xml_document, "examples/tdl.rng", File.read("spec/fixtures/invalid_template.tdl")) + errors = base_command.send(:validate_xml_document, "examples/tdl.rng", File.read("spec/fixtures/invalid_template.tdl")) errors.length.should > 0 end it "should return no errors" do - b = BaseCommand.new - errors = b.send(:validate_xml_document, "examples/tdl.rng", File.read("spec/fixtures/valid_template.tdl")) + errors = base_command.send(:validate_xml_document, "examples/tdl.rng", File.read("spec/fixtures/valid_template.tdl")) errors.length.should == 0 end end -- 1.7.7.4

--- examples/bad_image_description.xml | 3 + spec/command/import_command_spec.rb | 197 +++++++++++++++++++++++++++++----- 2 files changed, 171 insertions(+), 29 deletions(-) create mode 100644 examples/bad_image_description.xml diff --git a/examples/bad_image_description.xml b/examples/bad_image_description.xml new file mode 100644 index 0000000..82c3b15 --- /dev/null +++ b/examples/bad_image_description.xml @@ -0,0 +1,3 @@ +<badimage> + <name>Test Image</name> +</badimage> diff --git a/spec/command/import_command_spec.rb b/spec/command/import_command_spec.rb index bf3e922..1e8c5ee 100644 --- a/spec/command/import_command_spec.rb +++ b/spec/command/import_command_spec.rb @@ -17,43 +17,182 @@ require 'spec_helper' module Aeolus module CLI describe ImportCommand do - before(:each) do - @options = { :id => "ami-5592553c", - :target => ["ec2"], - :provider_account => ["ec2-us-east-1"], - :description => "<image><name>MyImage</name></image>"} - end + let( :description ) { "<image><name>MyImage</name></image>" } + let( :options ) { { :id => "ami-5592553c", + :target => ["ec2"], + :provider_account => ["ec2-us-east-1"], + :description => description } } + let( :importc ) { ImportCommand.new( options ) } describe "#import_image" do - it "should import an image with default description values" do - VCR.use_cassette('command/import_command/import_image_default') do - importc = ImportCommand.new(@options) - begin - importc.import_image - rescue SystemExit => e - e.status.should == 0 + context "without description parameter" do + let( :options ) { { :id => "ami-5592553c", + :target => ["ec2"], + :provider_account => ["ec2-us-east-1"] } } + + it "should import an image with default description value" do + VCR.use_cassette('command/import_command/import_image_default') do + begin + importc.import_image + rescue SystemExit => e + e.status.should == 0 + end + $stdout.string.should include("Image:") + $stdout.string.should include("Target Image:") + $stdout.string.should include("Build:") + $stdout.string.should include("Provider Image:") end - $stdout.string.should include("Image:") - $stdout.string.should include("Target Image:") - $stdout.string.should include("Build:") - $stdout.string.should include("Provider Image:") end end - it "should import an image with file description" do - VCR.use_cassette('command/import_command/import_image_description_file') do - @options[:description] = 'spec/sample_data/image_description.xml' - importc = ImportCommand.new(@options) - begin - importc.import_image - rescue SystemExit => e - e.status.should == 0 + context "with description parameter" do + context "as a string" do + context "in correct format" do + let( :description ) { "<image><name>MyImage</name></image>" } + it "should import an image with provided description value" do + VCR.use_cassette('command/import_command/import_image_default') do + begin + importc.import_image + rescue SystemExit => e + e.status.should == 0 + end + $stdout.string.should include("Image:") + $stdout.string.should include("Target Image:") + $stdout.string.should include("Build:") + $stdout.string.should include("Provider Image:") + end + end + end + + context "in incorrect format" do + let( :description ) { "<badimage><name>MyImage</name></badimage>" } + it "should not import an image at all" do + VCR.use_cassette('command/import_command/import_image_default') do + begin + importc.import_image + rescue SystemExit => e + e.status.should == 1 + end + $stdout.string.should include("ERROR") + $stdout.string.should include("Invalid description") + end + end end - $stdout.string.should include("Image:") - $stdout.string.should include("Target Image:") - $stdout.string.should include("Build:") - $stdout.string.should include("Provider Image:") end + + context "as a file" do + context "which exists" do + context "and contains correct xml" do + + let( :description ) { 'examples/image_description.xml' } + + it "should import an image with file description" do + VCR.use_cassette('command/import_command/import_image_description_file') do + begin + importc.import_image + rescue SystemExit => e + e.status.should == 0 + end + $stdout.string.should include("Image:") + $stdout.string.should include("Target Image:") + $stdout.string.should include("Build:") + $stdout.string.should include("Provider Image:") + end + end + + end + + context "and does not contain correct xml" do + + let( :description ) { 'examples/bad_image_description.xml' } + + it "should not import an image at all" do + VCR.use_cassette('command/import_command/import_image_description_file') do + begin + importc.import_image + rescue SystemExit => e + e.status.should == 1 + end + $stdout.string.should include("ERROR") + $stdout.string.should include("Invalid description") + end + end + + end + end + + context "which does not exists" do + + let( :description ) { 'examples/no_file_image_description.xml' } + + it "should not import an image at all" do + VCR.use_cassette('command/import_command/import_image_description_file') do + begin + importc.import_image + rescue SystemExit => e + e.status.should == 1 + end + $stdout.string.should include("ERROR") + $stdout.string.should include("Invalid description") + end + end + + end + end + end + end + + describe "#import_params_valid!" do + subject { lambda { importc.send( :import_params_valid!, params ) } } + context "correct params" do + let( :params ) { { :id => "ami-5592553c", + :target => ["ec2"], + :provider_account => ["ec2-us-east-1"], + :description => description } } + it { subject.call.should be_true } + end + + context "missing parameter" do + let( :params ) { { :id => "ami-5592553c", + :provider_account => ["ec2-us-east-1"], + :description => description } } + it { should raise_error(ArgumentError, /missing/) } + end + + context "unexpected parameter" do + let( :params ) { { :id => "ami-5592553c", + :target => ["ec2"], + :provider_account => ["ec2-us-east-1"], + :other_parameter => "other_value", + :description => description } } + it { should raise_error(ArgumentError, /unexpected/) } + end + + context "any parameter is nil" do + let( :params ) { { :id => "ami-5592553c", + :target => nil, + :provider_account => ["ec2-us-east-1"], + :other_parameter => "other_value", + :description => description } } + it { should raise_error(ArgumentError, /params should not contain nil/) } + end + + context "parameters is not a Hash" do + let( :params ) { "ami-5592553c" } + it { should raise_error(TypeError, /params should be Hash instead/) } + end + end + + describe "validate_description_xml!" do + subject { lambda { importc.send( :validate_description_xml!, xml ) } } + context "with correct xml" do + let( :xml ) { "<image><name>XML-NAME</name></image>" } + it { subject.call.should be_true } + end + + context "with incorrect xml" do + let( :xml ) { "<badimage><name>XML-NAME</name></badimage>" } + it { should raise_error( ArgumentError, /Invalid description/ ) } end end end -- 1.7.7.4