[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