[PATCH aeolus-cli] BZ#797157 Validate aeolus config

Jan Provaznik jprovazn at redhat.com
Thu Mar 1 13:09:56 UTC 2012


On 02/24/2012 01:21 PM, mtaylor at redhat.com wrote:
> From: Martyn Taylor<mtaylor at redhat.com>
>
> ---
>   lib/aeolus_cli.rb                      |    2 ++
>   lib/aeolus_cli/command/base_command.rb |   28 ++++++++++++++++++++--------
>   lib/aeolus_cli/exceptions.rb           |   28 ++++++++++++++++++++++++++++
>   spec/command/base_command_spec.rb      |    5 +++++
>   spec/spec_helper.rb                    |    1 +
>   5 files changed, 56 insertions(+), 8 deletions(-)
>   create mode 100644 lib/aeolus_cli/exceptions.rb
>
> diff --git a/lib/aeolus_cli.rb b/lib/aeolus_cli.rb
> index 8070845..2e6e88c 100644
> --- a/lib/aeolus_cli.rb
> +++ b/lib/aeolus_cli.rb
> @@ -33,3 +33,5 @@ require File.join(File.dirname(__FILE__), 'aeolus_cli/model', 'provider')
>   require File.join(File.dirname(__FILE__), 'aeolus_cli/model', 'provider_account')
>   require File.join(File.dirname(__FILE__), 'aeolus_cli/model', 'provider_type')
>   require File.join(File.dirname(__FILE__), 'aeolus_cli/model', 'environment')
> +
> +require File.join(File.dirname(__FILE__), 'aeolus_cli/', 'exceptions')
> diff --git a/lib/aeolus_cli/command/base_command.rb b/lib/aeolus_cli/command/base_command.rb
> index 422e5f4..b9584d6 100644
> --- a/lib/aeolus_cli/command/base_command.rb
> +++ b/lib/aeolus_cli/command/base_command.rb
> @@ -24,11 +24,15 @@ module Aeolus
>         attr_accessor :options
>
>         def initialize(opts={}, logger=nil)
> -        logger(logger)
> -        @options = opts
> -        @config_location = "~/.aeolus-cli"
> -        @config = load_config
> -        configure_active_resource
> +        begin
> +          logger(logger)
> +          @options = opts
> +          @config_location = "~/.aeolus-cli"
> +          @config = load_config
> +          configure_active_resource
> +        rescue =>  e
> +          handle_exception(e)
> +        end
>         end
>
>         protected
> @@ -115,6 +119,10 @@ module Aeolus
>             code = "Argument Error"
>             message = e.message
>
> +        elsif e.is_a?(Aeolus::CLI::ConfigError)
> +          code = "Config Error"
> +          message = e.message + "; please check ~/.aeolus-cli"
> +
>           elsif e.respond_to?(:response)
>             doc = Nokogiri::XML e.response.body
>             code = doc.xpath("/error/code").text
> @@ -167,9 +175,13 @@ module Aeolus
>         end
>
>         def configure_active_resource
> -        Aeolus::CLI::Base.site = @config[:conductor][:url]
> -        Aeolus::CLI::Base.user = @config[:conductor][:username]
> -        Aeolus::CLI::Base.password = @config[:conductor][:password]
> +        if @config.has_key?(:conductor)&&  @config[:conductor].keys.to_set.subset?([:url, :password, :username].to_set)
> +          Aeolus::CLI::Base.site = @config[:conductor][:url]
> +          Aeolus::CLI::Base.user = @config[:conductor][:username]
> +          Aeolus::CLI::Base.password = @config[:conductor][:password]
> +        else
> +          raise Aeolus::CLI::ConfigError.new("Error in configuration file")
> +        end

If I comment out :url key, so it looks:
:conductor:
   #:url: http://localhost:3000/api
   :username: admin
   :password: password

then I get:
$ aeolus-image list --images

ERROR:  Unknown Error => undefined method `path' for nil:NilClass
/usr/lib/ruby/gems/1.8/gems/activeresource-3.0.10/lib/active_resource/base.rb:562:in 
`prefix'
...


Could this be extended to print which config key is missing? Something 
like this:

raise Aeolus::CLI::ConfigError.new("#{key} is missing") unless 
@config.has_key?(:conductor)
[:url, :password, :username].each do |key|
   raise Aeolus::CLI::ConfigError.new("#{key} is missing") unless 
@config[:conductor].has_key?(key)
end

>         end
>
>         # Print Collection to STDOUT in a column layout
> diff --git a/lib/aeolus_cli/exceptions.rb b/lib/aeolus_cli/exceptions.rb
> new file mode 100644
> index 0000000..a523266
> --- /dev/null
> +++ b/lib/aeolus_cli/exceptions.rb
> @@ -0,0 +1,28 @@
> +#
> +#   Copyright 2011 Red Hat, Inc.
> +#
> +#   Licensed under the Apache License, Version 2.0 (the "License");
> +#   you may not use this file except in compliance with the License.
> +#   You may obtain a copy of the License at
> +#
> +#       http://www.apache.org/licenses/LICENSE-2.0
> +#
> +#   Unless required by applicable law or agreed to in writing, software
> +#   distributed under the License is distributed on an "AS IS" BASIS,
> +#   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> +#   See the License for the specific language governing permissions and
> +#   limitations under the License.
> +#
> +module Aeolus
> +  module CLI
> +    class Error<  StandardError
> +      attr_reader :message
> +
> +      def initialize(message)
> +        @message  = message
> +      end
> +    end

Any reason for Error definition? Isn't sufficient this?

class ConfigError<  StandardError; end

> +
> +    class ConfigError<  Error; end
> +  end
> +end
> \ No newline at end of file
> diff --git a/spec/command/base_command_spec.rb b/spec/command/base_command_spec.rb
> index 8c1ef5f..f71f91a 100644
> --- a/spec/command/base_command_spec.rb
> +++ b/spec/command/base_command_spec.rb
> @@ -26,6 +26,11 @@ module Aeolus
>             Aeolus::CLI::Base.user.to_s.should == "admin"
>             Aeolus::CLI::Base.password.to_s.should == "password"
>           end
> +
> +        it "should raise error on invalid config" do
> +          base_command.instance_eval { @config = {} }
> +          lambda { base_command.send(:configure_active_resource) }.should raise_error(Aeolus::CLI::ConfigError, "Error in configuration file")
> +        end
>         end
>
>         describe "#read_file" do
> diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
> index d198732..368c18f 100644
> --- a/spec/spec_helper.rb
> +++ b/spec/spec_helper.rb
> @@ -14,6 +14,7 @@
>
>   $:<<  File.expand_path(File.join(File.dirname(__FILE__), "../lib/aeolus_cli/command/"))
>   $:<<  File.expand_path(File.join(File.dirname(__FILE__), "../lib/aeolus_cli/model/"))
> +$:<<  File.expand_path(File.join(File.dirname(__FILE__), "../lib/aeolus_cli/"))
>
>   require 'aeolus_cli'
>   require 'stringio'

Hi Martyn,
couple of minor notes inline.

Jan



More information about the aeolus-devel mailing list