Hi Greg, Overall this looks and works great. There are several things to point out, though.

The following files have trailing whitespace: src/features/config_server.feature src/features/step_definitions/config_server_steps.rb src/spec/controllers/config_servers_controller_spec.rb src/spec/models/config_server_spec.rb We don't let trailing spaces or empty lines at the EOF into the repo. Could you please remove them?

If you config your repo according to this: http://aeolusproject.org/page/Git_Configuration git will bitch about trailing whitespace before every commit.

The remaining comments on the code are inline.

Thanks for this! Feel free to reply here or ping me on IRC (shadow) if you have any questions. Cheers, Thomas -- No trees were killed to send this message, but a large number of electrons were terribly inconvenienced. On 06/24/2011 05:18 PM, Greg Blomquist wrote: > > This commit adds the ability to manage config server data as an object > > associated to provider accounts. Specifically, provider accounts can have > > 0-or-1 config servers. > > > > Now, a user can add, edit, delete, and test a config server from the provider account > > view. > > > > Signed-off-by: Greg Blomquist&lt;gblomqui(a)redhat.com&gt; > > --- > > src/app/controllers/config_servers_controller.rb | 73 ++++++++ > > src/app/models/config_server.rb | 106 +++++++++++ > > src/app/models/provider_account.rb | 3 + > > .../views/config_servers/_config_server_form.haml | 22 +++ > > src/app/views/config_servers/_section_header.haml | 3 + > > src/app/views/config_servers/edit.haml | 21 +++ > > src/app/views/config_servers/new.haml | 16 ++ > > src/app/views/provider_accounts/_properties.haml | 22 +++ > > src/config/locales/en.yml | 15 ++ > > src/config/routes.rb | 1 + > > .../migrate/20110606141425_create_config_server.rb | 18 ++ > > src/features/config_server.feature | 82 +++++++++ > > .../step_definitions/config_server_steps.rb | 28 +++ > > src/features/support/env.rb | 3 + > > src/features/support/paths.rb | 8 +- > > .../controllers/config_servers_controller_spec.rb | 187 ++++++++++++++++++++ > > src/spec/factories/config_server.rb | 36 ++++ > > src/spec/models/config_server_spec.rb | 42 +++++ > > src/spec/models/provider_account_spec.rb | 10 + > > 19 files changed, 695 insertions(+), 1 deletions(-) > > create mode 100644 src/app/controllers/config_servers_controller.rb > > create mode 100644 src/app/models/config_server.rb > > create mode 100644 src/app/views/config_servers/_config_server_form.haml > > create mode 100644 src/app/views/config_servers/_section_header.haml > > create mode 100644 src/app/views/config_servers/edit.haml > > create mode 100644 src/app/views/config_servers/new.haml > > create mode 100644 src/db/migrate/20110606141425_create_config_server.rb > > create mode 100644 src/features/config_server.feature > > create mode 100644 src/features/step_definitions/config_server_steps.rb > > create mode 100644 src/spec/controllers/config_servers_controller_spec.rb > > create mode 100644 src/spec/factories/config_server.rb > > create mode 100644 src/spec/models/config_server_spec.rb > > > > diff --git a/src/app/controllers/config_servers_controller.rb b/src/app/controllers/config_servers_controller.rb > > new file mode 100644 > > index 0000000..9f95e5c > > --- /dev/null > > +++ b/src/app/controllers/config_servers_controller.rb > > @@ -0,0 +1,73 @@ > > +require 'rest_client' > > + > > +class ConfigServersController< ApplicationController > > + before_filter :require_user > > + before_filter :set_view_vars, :only => [:index,:show] The `set_view_vars` method is not defined here, I suspect it's just a copy&paste artifact. If so, please remove it.

> > + layout 'application' > > + > > + def top_section > > + :administer > > + end > > + > > + def edit > > + @config_server = ConfigServer.find(params[:id]) > > + @provider_account = @config_server.provider_account > > + end > > + > > + def new > > + @provider_account = ProviderAccount.find(params[:provider_account_id]) > > + @config_server = ConfigServer.new() > > + end > > + > > + def test > > + config_server = ConfigServer.find(params[:id]) > > + if error = config_server.validate_connection > > + flash[:error] = error > > + else > > + flash[:notice] = "Test successful" > > + end > > + redirect_to provider_account_path(config_server.provider_account.id) > > + end > > + > > + def create > > + @provider_account = ProviderAccount.find(params[:provider_account_id]) > > + # for now the privileges required to create, modify, or delete a config > > + # server are tied to modifying the particular associated provider account > > + require_privilege(Privilege::MODIFY, @provider_account) > > + > > + @config_server = ConfigServer.new(params[:config_server]) > > + @config_server.provider_account = @provider_account > > + if @config_server.invalid? > > + flash[:error] = "The config server information is invalid." > > + render :action => 'new' and return > > + end > > + @config_server.save! > > + flash[:notice] = "Config server added." > > + redirect_to provider_account_path(@provider_account) > > + end > > + > > + def update > > + @config_server = ConfigServer.find(params[:id]) > > + @provider_account = @config_server.provider_account > > + require_privilege(Privilege::MODIFY, @provider_account) > > + > > + if @config_server.update_attributes(params[:config_server]) > > + flash[:notice] = "Config server updated." > > + redirect_to provider_account_path(@provider_account) > > + else > > + flash[:error] = "Config server was not updated" > > + render :action => :edit > > + end > > + end > > + > > + def destroy > > + @config_server = ConfigServer.find(params[:id]) > > + require_privilege(Privilege::MODIFY, @config_server.provider_account) > > + if ConfigServer.destroy(params[:id]) > > + flash[:notice] = "Config server was deleted." > > + else > > + flash[:error] = "Config server was not deleted" > > + end > > + redirect_to provider_account_path((a)config_server.provider_account) > > + end > > +end > > diff --git a/src/app/models/config_server.rb b/src/app/models/config_server.rb > > new file mode 100644 > > index 0000000..050642e > > --- /dev/null > > +++ b/src/app/models/config_server.rb > > @@ -0,0 +1,106 @@ > > +# == Schema Information > > +# Schema version: 20110517095823 > > +# > > +# Table name: config_servers > > +# > > +# id :integer not null, primary key > > +# host :string(255) not null > > +# port :string(255) not null > > +# username :string(255) null > > +# password :string(255) null > > +# certificate :string(255) null > > +# provider_id :integer not null, fk_provider > > +# > > + > > +## Copyright (C) 2011 Red Hat, Inc. > > +## Written by Greg Blomquist&lt;gblomqui(a)redhat.com&gt; > > +## > > +## This program is free software; you can redistribute it and/or modify > > +## it under the terms of the GNU General Public License as published by > > +## the Free Software Foundation; version 2 of the License. > > +## > > +## This program is distributed in the hope that it will be useful, > > +## but WITHOUT ANY WARRANTY; without even the implied warranty of > > +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +## GNU General Public License for more details. > > +## > > +## You should have received a copy of the GNU General Public License > > +## along with this program; if not, write to the Free Software > > +## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, > > +## MA 02110-1301, USA. A copy of the GNU General Public License is > > +## also available at http://www.gnu.org/copyleft/gpl.html. > > +# > > + > > +class ConfigServer< ActiveRecord::Base > > + > > + belongs_to :provider_account > > + > > + validates_presence_of :host > > + validates_presence_of :port > > + validate :validate_url > > + > > + def base_url > > + scheme = (certificate.nil? or certificate.empty?) ? "http" : "https" You can replace the nil? and empty? checks with a single `certificate.blank?` check.

> > + domain = host > > + if not (port.nil? or port.empty?) Same here ^ port.blank?

> > + domain += ":#{port}" > > + end > > + "#{scheme}://#{domain}" > > + end > > + > > + # Validates the connection to this config server. Returns an error string if > > + # the connection cannot be validated. Otherwise, returns nil. > > + # > > + # Example: > > + # > > + # if nil == error = config_server.validate_connection > > + # puts "connection successful" > > + # else > > + # puts "connection unsuccessful: #{error}" > > + # end > > + # > > + def validate_connection > > + error_str = nil > > + begin > > + response = test_connection > > + if response.code != 200 > > + error_str = I18n.translate("config_servers.errors.connection.unhandled_response", :url => base_url, :code => response.code) > > + end > > + rescue => e > > + error_str = map_connection_exception_to_error(e) > > + end > > + return error_str > > + end I'm afraid this may get a bit confusing in the typical ruby use: if my_config_server.validate_connection # it's invalid here! else # valid here end How about renaming the method to `connection_error_msg` and adding a `connection_valid?` method that behaves idiomatically?

Folks cold get tripped on this. > > + > > + def validate_url > > + error_string = validate_connection > > + if not error_string.nil? > > + errors.add_to_base(error_string) > > + end > > + end > > + > > + private > > + def test_connection > > + args = {:method => :get} > > + if not (username.nil? or username.empty?) username.blank? ^

> > + args[:user] = username > > + args[:password] = password > > + end > > + if not (certificate.nil? or certificate.empty?) certificate.blank? ^

> > + args[:ssl_client_cert] = OpenSSL::X509::Certificate.new(certificate) > > + end > > + args[:url] = "#{base_url}/version" > > + args[:raw_response] = true > > + RestClient::Request.execute(args) > > + end > > + > > + def map_connection_exception_to_error(ex) > > + if ex.class == OpenSSL::X509::CertificateError > > + error_string = I18n.translate("config_servers.errors.connection.certificate", :url => base_url, :msg => ex.message) > > + elsif ex.kind_of?(RestClient::ExceptionWithResponse) or ex.kind_of?(RestClient::Exception) or ex.class == Errno::ETIMEDOUT > > + error_string = I18n.translate("config_servers.errors.connection.generic_with_message", :url => base_url, :msg => ex.message) > > + elsif not ex.nil? > > + error_string = I18n.translate("config_servers.errors.connection.generic", :url => base_url) > > + end > > + end > > +end > > diff --git a/src/app/models/provider_account.rb b/src/app/models/provider_account.rb > > index 88ebe5b..f86727d 100644 > > --- a/src/app/models/provider_account.rb > > +++ b/src/app/models/provider_account.rb > > @@ -57,6 +57,8 @@ class ProviderAccount< ActiveRecord::Base > > # validation of credentials is done in provider_account validation, :validate => false prevents nested_attributes from validation > > has_many :credentials, :dependent => :destroy, :validate => false > > accepts_nested_attributes_for :credentials > > + # eventually, this might be "has_many", but first pass is one-to-one > > + has_one :config_server, :dependent => :destroy > > > > # Helpers > > attr_accessor :x509_cert_priv_file, :x509_cert_pub_file > > @@ -118,6 +120,7 @@ class ProviderAccount< ActiveRecord::Base > > > > def destroyable? > > instances.empty? > > + config_server.nil? > > end This ^ overrides the `instances.empty?` check which needs to be here (at least for now). This makes one cucumber and one rspec test fail. You could do this instead: def destroyable? instances.empty? and config_server.nil? end However, is that check necessary at all? The config_server is `dependent => :destroy` anyway, so the provider account is always destroyable in that regard.

The reason we've put the check for empty instances here was that otherwise the admin could remove an account that still had instances hung onto it. These instances could be running and rack up the bill unbeknownst to Conductor. > > > > def connect > > diff --git a/src/app/views/config_servers/_config_server_form.haml b/src/app/views/config_servers/_config_server_form.haml > > new file mode 100644 > > index 0000000..5e60896 > > --- /dev/null > > +++ b/src/app/views/config_servers/_config_server_form.haml > > @@ -0,0 +1,22 @@ > > +%fieldset.clearfix > > + - if not @config_server.id.nil? > > + = hidden_field_tag "config_server[id]", @config_server.id > > + = hidden_field_tag "provider_account_id", @provider_account.id > > + = label_tag "config_server[host]", "Host: " > > + = text_field_tag "config_server[host]", @config_server.host > > +%fieldset.clearfix > > + = label_tag "config_server[port]", "Port: " > > + = text_field_tag "config_server[port]", @config_server.port > > +%fieldset.clearfix > > + = label_tag "config_server[username]", "Username: " > > + = text_field_tag "config_server[username]", @config_server.username > > +%fieldset.clearfix > > + = label_tag "config_server[password]", "Password: " > > + = password_field_tag "config_server[password]", @config_server.password > > +%fieldset.clearfix > > + = label_tag "config_server[certificate]", "Certificate: " > > + = text_area_tag "config_server[certificate]", @config_server.certificate, :rows => 20, :cols => 64 > > + = t('config_servers.certificate_help') > > +%fieldset.clearfix > > + .grid_13.alpha.omega > > + = submit_tag t(:save), :class => "ra nomargin dialogbutton" > > diff --git a/src/app/views/config_servers/_section_header.haml b/src/app/views/config_servers/_section_header.haml > > new file mode 100644 > > index 0000000..d8e75fe > > --- /dev/null > > +++ b/src/app/views/config_servers/_section_header.haml > > @@ -0,0 +1,3 @@ > > +%header.page-header > > + %h1{:class => controller.controller_name} Config Server > > + .corner&nbsp; > > diff --git a/src/app/views/config_servers/edit.haml b/src/app/views/config_servers/edit.haml > > new file mode 100644 > > index 0000000..42e40f8 > > --- /dev/null > > +++ b/src/app/views/config_servers/edit.haml > > @@ -0,0 +1,21 @@ > > += render :partial => 'layouts/admin_header' > > +%header.page-header > > + %h1{:class => controller.controller_name}= &quot;#{(a)provider_account.name} Config Server" > > + #obj_actions.button-container > > + %div.button-group > > + = link_to 'Cancel Editing', provider_account_path(@provider_account), :class => 'button pill danger', :id => 'cancel_config_server_edit' > > + .corner&nbsp; > > + > > +%section.content-section.config_server > > + %header > > + %h2 Edit Config Server > > + > > + .content > > + = error_messages_for :config_server > > + %h2 > > + = t('config_servers.edit.edit_config_server') > > + - form_tag(config_server_path, { :method => :put }) do > > + = render :partial => 'config_server_form', :locals => {:from => "edit"} > > + -#%fieldset.clearfix > > + .grid_13.alpha.omega > > + = submit_tag t(:save), :class => "ra nomargin dialogbutton" > > diff --git a/src/app/views/config_servers/new.haml b/src/app/views/config_servers/new.haml > > new file mode 100644 > > index 0000000..c21aee4 > > --- /dev/null > > +++ b/src/app/views/config_servers/new.haml > > @@ -0,0 +1,16 @@ > > += render :partial => 'layouts/admin_header' > > += render :partial => 'section_header' > > + > > +%section.content-section.config_server > > + %header > > + %h2 New Config Server > > + > > + .content > > + = error_messages_for :config_server > > + %h2 > > + = t('config_servers.new.new_config_server') > > + - form_for(@config_server, :url => config_servers_path) do |f| > > + = render :partial => 'config_server_form', :locals => {:from => "new"} > > + -#%fieldset > > + .grid_13.alpha.omega > > + = f.submit t(:add), :class => "ra nomargin dialogbutton" > > diff --git a/src/app/views/provider_accounts/_properties.haml b/src/app/views/provider_accounts/_properties.haml > > index 559bdf1..6990a17 100644 > > --- a/src/app/views/provider_accounts/_properties.haml > > +++ b/src/app/views/provider_accounts/_properties.haml > > @@ -17,3 +17,25 @@ > > = label_tag "Account number:" > > %td > > = @account_id > > + %tr > > + %td > > + %label Config Server: > > + %td > > + - missing_config_server = @account.config_server.nil? > > + %span#config_server > > + = missing_config_server ? "None" : @account.config_server.base_url > > + %span#config_server_control > > + - if missing_config_server > > + [ > > + = link_to 'Add', new_config_server_url + &quot;?provider_account_id=#{(a)account.id}&quot; > > + ] > > + - else > > + [ > > + = link_to 'Edit', edit_config_server_path((a)account.config_server) > > + ] > > + [ > > + = link_to 'Test', test_config_servers_path + &quot;?id=#{(a)account.config_server.id}&quot; > > + ] > > + [ > > + = link_to 'Delete', config_server_path((a)account.config_server), :method => 'delete', :confirm => "Are you sure you want to delete this config server?" > > + ] > > diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml > > index 021cffa..1018f91 100644 > > --- a/src/config/locales/en.yml > > +++ b/src/config/locales/en.yml > > @@ -192,6 +192,9 @@ en: > > account_not_deleted: > > one: "Account %{list} was not deleted. There are instances associated with it." > > other: "Accounts %{list} were not deleted. They have instances associated with them." > > + config_server_saved: "Config Server data was successfully saved" > > + config_server_not_saved: "An error occurred while saving the Config Server data." > > + > > new: > > new_provider_account: New Account > > required_field: Required field. > > @@ -209,4 +212,16 @@ en: > > account_number: AWS Account ID > > account_private_cert: EC2 x509 private key > > account_public_cert: EC2 x509 public key > > + config_servers: > > + certificate_help: Provide a certificate to enable https support > > + new: > > + new_config_server: New Config Server > > + edit: > > + edit_config_server: Edit Config Server > > + errors: > > + connection: > > + generic: "Could not validate config server connection (url: %{url}). Check the connection parameters and try again." > > + generic_with_message: "Could not validate config server connection (url: %{url}). %{msg}" > > + certificate: "Could not validate config server connection (url: %{url}). Certificate error: %{msg}" > > + unhandled_response: "Could not validate config server connection (url: %{url}). HTTP Status Code: %{code}" > > uptime: Uptime > > diff --git a/src/config/routes.rb b/src/config/routes.rb > > index 7f83338..18ae28d 100644 > > --- a/src/config/routes.rb > > +++ b/src/config/routes.rb > > @@ -46,6 +46,7 @@ ActionController::Routing::Routes.draw do |map| > > map.resources :provider_types, :only => :index > > map.resources :users, :collection => { :multi_destroy => :delete } > > map.resources :provider_accounts, :collection => { :multi_destroy => :delete, :set_selected_provider => :get} > > + map.resources :config_servers, :collection => { :test => :get } > > map.resources :roles, :collection => { :multi_destroy => :delete } > > map.resources :settings, :collection => { :self_service => :get, :general_settings => :get } > > map.resources :pool_families, :collection => { :multi_destroy => :delete, :add_provider_account => :post, :multi_destroy_provider_accounts => :delete } > > diff --git a/src/db/migrate/20110606141425_create_config_server.rb b/src/db/migrate/20110606141425_create_config_server.rb > > new file mode 100644 > > index 0000000..e10ce78 > > --- /dev/null > > +++ b/src/db/migrate/20110606141425_create_config_server.rb > > @@ -0,0 +1,18 @@ > > +class CreateConfigServer< ActiveRecord::Migration > > + def self.up > > + create_table :config_servers do |t| > > + t.string :host, :null => false > > + t.string :port, :null => false > > + t.string :username, :null => true > > + t.string :password, :null => true > > + t.string :certificate, :null => true, :limit => 2048 > > + t.integer :provider_account_id, :null => false > > + > > + t.timestamps > > + end > > + end > > + > > + def self.down > > + drop_table :config_servers > > + end > > +end > > diff --git a/src/features/config_server.feature b/src/features/config_server.feature > > new file mode 100644 > > index 0000000..73d3f6d > > --- /dev/null > > +++ b/src/features/config_server.feature > > @@ -0,0 +1,82 @@ > > +Feature: Config Servers > > + In order to administer configuration management on systems > > + As a user > > + I want to manage config servers as part of provider accounts > > + > > + Background: > > + Given I am an authorised user > > + And I am logged in > > + > > + # This scenario relies on a stubbed version of ConfigServer > > + # that never fails on the connection test > > + Scenario: I am able to add a config server to a provider account > > + Given I am on the homepage > > + And there is mock provider account "mock_account" > > + And I want to add a new config server > > + When I go to mock_account's provider account page > > + Then I should see "None" within "#config_server" > > + And I should see "[ Add ]" within "#config_server_control" > > + When I follow "Add" > > + Then I should be on the new config server page > > + When I fill in "config_server[host]" with "valid_host" > > + When I fill in "config_server[port]" with "valid_port" > > + And I press "Save" > > + Then I should be on mock_account's provider account page > > + And I should see "Config server added" > > + And I should see "[ Edit ]" within "#config_server_control" > > + > > + # This is essentially the same scenario as the first, but creates > > + # a different stubbed ConfigServer, so it fails > > + Scenario: I cannot add a config server with invalid host or port information > > + Given I am on the homepage > > + And there is mock provider account "mock_account" > > + And I am not sure about the config server host > > + When I go to mock_account's provider account page > > + Then I should see "None" within "#config_server" > > + And I should see "[ Add ]" within "#config_server_control" > > + When I follow "Add" > > + Then I should be on the new config server page > > + When I fill in "config_server[host]" with "invalid" > > + When I fill in "config_server[port]" with "invalid" > > + And I press "Save" > > + Then I should see "The config server information is invalid" > > + And I should see "Could not validate config server connection" > > + > > + # This is essentially the same scenario as the first, but creates > > + # a different stubbed ConfigServer, so it fails > > + Scenario: I cannot add a config server with invalid credentials > > + Given I am on the homepage > > + And there is mock provider account "mock_account" > > + And I am not sure about the config server credentials > > + When I go to mock_account's provider account page > > + Then I should see "None" within "#config_server" > > + And I should see "[ Add ]" within "#config_server_control" > > + When I follow "Add" > > + Then I should be on the new config server page > > + When I fill in "config_server[host]" with "valid" > > + When I fill in "config_server[port]" with "valid" > > + When I fill in "config_server[username]" with "invalid" > > + When I fill in "config_server[password]" with "invalid" > > + And I press "Save" > > + Then I should see "The config server information is invalid" > > + And I should see "Could not validate config server connection" > > + > > + Scenario: I should be able to edit a config server > > + Given I am on the homepage > > + And there is a mock config server "https://mock:443" for account "mock_account" > > + When I go to mock_account's provider account page > > + Then I should see "[ Edit ]" within "#config_server_control" > > + When I follow "Edit" within "#config_server_control" > > + Then I should be on the edit config server page for account "mock_account" > > + And I press "Save" > > + Then I should be on mock_account's provider account page > > + And I should see "Config server updated" > > + > > + Scenario: I should be able to delete an existing config server > > + Given I am on the homepage > > + And there is a mock config server "https://mock:443" for account "mock_account" > > + When I go to mock_account's provider account page > > + Then I should see "[ Delete ]" within "#config_server_control" > > + When I follow "Delete" within "#config_server_control" > > + Then I should see "Config server was deleted" > > + And I should be on mock_account's provider account page > > diff --git a/src/features/step_definitions/config_server_steps.rb b/src/features/step_definitions/config_server_steps.rb > > new file mode 100644 > > index 0000000..ec55076 > > --- /dev/null > > +++ b/src/features/step_definitions/config_server_steps.rb > > @@ -0,0 +1,28 @@ > > +Given /^I want to add a new config server$/ do > > + c = Factory.build(:mock_config_server) > > + ConfigServer.stub!(:new).and_return(c) > > +end > > + > > +Given /^I am not sure about the config server (host|port)$/ do |host_or_port| > > + c = Factory.build(:invalid_host_or_port_config_server) > > + ConfigServer.stub!(:new).and_return(c) > > +end > > + > > +Given /^I am not sure about the config server credentials$/ do > > + c = Factory.build(:invalid_credentials_config_server) > > + ConfigServer.stub!(:new).and_return(c) > > +end > > + > > +Given /^there is a mock config server "(http|https):\/\/(.*):(.*)" for account "(.*)"$/ do |scheme,host,port,acc| > > + provider = Factory :mock_provider, :name => "mock_provider" > > + mock_account = Factory :mock_provider_account, :label => acc, :provider => provider > > + params = {:host => host, :port => port, :provider_account => mock_account} > > + if "https" == scheme > > + params[:certificate] = "cert" > > + end > > + @config_server = Factory :mock_config_server, params > > + # ensure that the :mock_config_server (with stubbed methods) is returned > > + ConfigServer.stub!(:find).and_return(@config_server) > > +end > > + > > + > > diff --git a/src/features/support/env.rb b/src/features/support/env.rb > > index 33f9ed2..7357eda 100644 > > --- a/src/features/support/env.rb > > +++ b/src/features/support/env.rb > > @@ -19,6 +19,9 @@ require File.expand_path(File.dirname(__FILE__) + '../../../spec/vcr_setup.rb') > > require 'webrat' > > require 'webrat/core/matchers' > > > > +# Pull in the ability to stub out methods > > +require 'spec/stubs/cucumber' > > + > > Webrat.configure do |config| > > config.mode = :rails > > config.open_error_files = false # Set to true if you want error pages to pop up in the browser > > diff --git a/src/features/support/paths.rb b/src/features/support/paths.rb > > index 12a72d5..f220e3d 100644 > > --- a/src/features/support/paths.rb > > +++ b/src/features/support/paths.rb > > @@ -98,6 +98,13 @@ module NavigationHelpers > > when /^(.*)'s provider account page$/ > > provider_account_path(ProviderAccount.find_by_label($1)) > > > > + when /the new config server page/ > > + url_for new_config_server_path > > + > > + when /^the edit config server page/ > > + @config_server.stub!(:validate_connection).and_return(nil) > > + edit_config_server_path(@config_server) > > + > > when /the operational status of deployment page/ > > deployment_path(@deployment, :details_tab => 'operation') > > > > @@ -123,7 +130,6 @@ module NavigationHelpers > > when /^the (.*)'s edit user page$/ > > edit_user_path(User.find_by_login($1)) > > > > - > > # Add more mappings here. > > # Here is an example that pulls values out of the Regexp: > > # > > diff --git a/src/spec/controllers/config_servers_controller_spec.rb b/src/spec/controllers/config_servers_controller_spec.rb > > new file mode 100644 > > index 0000000..a9b6333 > > --- /dev/null > > +++ b/src/spec/controllers/config_servers_controller_spec.rb > > @@ -0,0 +1,187 @@ > > +require 'spec_helper' > > + > > +describe ConfigServersController do > > + fixtures :all > > + > > + before(:each) do > > + @admin_permission = Factory :admin_permission > > + @admin = @admin_permission.user > > + activate_authlogic > > + @session = UserSession.create(@admin) > > + end > > + > > + context "editing config servers" do > > + before(:each) do > > + @config_server = Factory :mock_config_server > > + @provider_account = @config_server.provider_account > > + end > > + > > + it "should provide UI to edit an existing Config Server" do > > + get :edit, :id => @config_server.id > > + response.should be_success > > + response.should render_template("edit") > > + end > > + > > + it "should allow users with account modify permissions to update a Config Server" do > > + post :update, :id => @config_server.id, :config_server => {:host => "host", :port => "port"} > > + response.should be_success > > + end > > + end > > + > > + context "creating config servers" do > > + before(:each) do > > + @provider_account = Factory :mock_provider_account > > + end > > + > > + it "should provide UI to create a new Config Server" do > > + get :new, :provider_account_id => @provider_account.id > > + response.should be_success > > + response.should render_template("new") > > + end > > + > > + it "should allow users with account modify permissions to create a Config Server" do > > + config_server = Factory :mock_config_server, :host => "host", :port => "port" > > + ConfigServer.stub!(:new).and_return(config_server) > > + post :create, :provider_account_id => @provider_account.id, > > + :config_server => { > > + :host => "host", > > + :port => "port" > > + } > > + response.should redirect_to(provider_account_path((a)provider_account.id)) > > + response.flash[:error].should be_nil > > + end > > + > > + it "should fail creating a config server when the username or password is invalid" do > > + config_server = Factory :invalid_credentials_config_server, :host => "host", :port => "port", :username => "invalid", :password => "invalid" > > + ConfigServer.stub!(:new).and_return(config_server) > > + post :create, :provider_account_id => @provider_account.id, > > + :config_server => { > > + :host => "host", > > + :port => "port", > > + :username => "invalid", > > + :password => "invalid" > > + } > > + response.should be_success > > + response.should render_template("new") > > + response.flash[:error].should == "The config server information is invalid." > > + end > > + > > + it "should fail creating a config server when the host and port are invalid" do > > + config_server = Factory :invalid_host_or_port_config_server, :host => "invalid", :port => "invalid" > > + ConfigServer.stub!(:new).and_return(config_server) > > + post :create, :provider_account_id => @provider_account.id, > > + :config_server => { > > + :host => "invalid", > > + :port => "invalid" > > + } > > + response.should be_success > > + response.should render_template("new") > > + response.flash[:error].should == "The config server information is invalid." > > + end > > + > > + it "should require that port is provided" do > > + post :create, :provider_account_id => @provider_account.id, > > + :config_server => { > > + :host => "host", > > + :port => "" > > + } > > + response.should be_success > > + response.should render_template("new") > > + response.flash[:error].should == "The config server information is invalid." > > + end > > + > > + it "should require that host is provided" do > > + post :create, :provider_account_id => @provider_account.id, > > + :config_server => { > > + :host => "", > > + :port => "port" > > + } > > + response.should be_success > > + response.should render_template("new") > > + response.flash[:error].should == "The config server information is invalid." > > + end > > + end > > +end > > + The commented-out section below should go, I think.

> > +=begin > > +describe ProviderAccountsController do > > + > > + fixtures :all > > + before(:each) do > > + @provider_account = Factory :mock_provider_account > > + @provider = @provider_account.provider > > + > > + @admin_permission = Permission.create :role => Role.find(:first, :conditions => ['name = ?', 'Provider Administrator']), > > + :permission_object => @provider, > > + :user => Factory(:provider_admin_user) > > + @admin = @admin_permission.user > > + activate_authlogic > > + end > > + > > + it "shows provider accounts as list" do > > + UserSession.create(@admin) > > + get :index, :provider_id => @provider.id > > + response.should be_success > > + response.should render_template("index") > > + end > > + > > + it "doesn't allow to save provider's account if not valid credentials" do > > + UserSession.create(@admin) > > + post :create, :provider_account => {:provider_id => @provider.id} > > + response.should be_success > > + response.should render_template("new") > > + response.flash[:error].should == "Credentials are invalid!" > > + end > > + > > + it "should permit users with account modify permission to access edit cloud account interface" do > > + UserSession.create(@admin) > > + get :edit, :id => @provider_account.id > > + response.should be_success > > + response.should render_template("edit") > > + end > > + > > + it "should allow users with account modify password to update a cloud account" do > > + UserSession.create(@admin) > > + @provider_account.credentials_hash = {:username => 'mockuser2', :password => "foobar"} > > + @provider_account.stub!(:valid_credentials?).and_return(true) > > + @provider_account.quota = Quota.new > > + @provider_account.save.should be_true > > + post :update, :id => @provider_account.id, :provider_account => { :credentials_hash => {:username => 'mockuser', :password => 'mockpassword'} } > > + response.should redirect_to provider_account_path(@provider_account) > > + ProviderAccount.find((a)provider_account.id).credentials_hash[&#39;password&#39;].should == "mockpassword" > > + end > > + > > + it "should allow users with account modify permission to delete a cloud account" do > > + UserSession.create(@admin) > > + lambda do > > + post :multi_destroy, :accounts_selected => [@provider_account.id] > > + end.should change(ProviderAccount, :count).by(-1) > > + response.should redirect_to provider_accounts_url > > + ProviderAccount.find_by_id((a)provider_account.id).should be_nil > > + end > > + > > + it "should deny access to users without account modify permission" do > > + get :edit, :id => @provider_account.id > > + response.should_not be_success > > + > > + post :update, :id => @provider_account.id, :provider_account => { :password => 'foobar' } > > + response.should_not be_success > > + > > + post :destroy, :id => @provider_account.id > > + response.should_not be_success > > + end > > + > > + it "should provide ui to create new account" do > > + UserSession.create(@admin) > > + get :new, :provider_id => @provider.id > > + response.should be_success > > + response.should render_template("new") > > + end > > + > > + it "should fail to grant access to account UIs for unauthenticated user" do > > + get :new > > + response.should_not be_success > > + end > > + > > +end > > +=end > > diff --git a/src/spec/factories/config_server.rb b/src/spec/factories/config_server.rb > > new file mode 100644 > > index 0000000..cfde6c3 > > --- /dev/null > > +++ b/src/spec/factories/config_server.rb > > @@ -0,0 +1,36 @@ > > +Factory.define :config_server do |f| > > + f.sequence(:host) {|n| "config_server#{n}" } > > +end > > + > > +Factory.define :mock_config_server, :parent => :config_server do |f| > > + f.association :provider_account, :factory => :mock_provider_account > > + f.host "localhost" > > + f.port "80" > > + f.username "username" > > + f.password "password" > > + f.after_build do |cs| > > + cs.stub!(:validate_connection).and_return(nil) if cs.respond_to? :stub! > > + end > > +end > > + > > +Factory.define :invalid_credentials_config_server, :parent => :config_server, :default_strategy => :build do |f| > > + f.host "localhost" > > + f.port "443" > > + f.username "bad_username" > > + f.password "bad_password" > > + f.certificate "cert" > > + f.after_build do |cs| > > + cs.stub!(:test_connection).and_raise(RestClient::Unauthorized) if cs.respond_to? :stub! > > + end > > +end > > + > > +Factory.define :invalid_host_or_port_config_server, :parent => :config_server, :default_strategy => :build do |f| > > + f.host "localhost" > > + f.port "443" > > + f.username "bad_username" > > + f.password "bad_password" > > + f.certificate "cert" > > + f.after_build do |cs| > > + cs.stub!(:test_connection).and_raise(Errno::ETIMEDOUT) if cs.respond_to? :stub! > > + end > > +end > > diff --git a/src/spec/models/config_server_spec.rb b/src/spec/models/config_server_spec.rb > > new file mode 100644 > > index 0000000..857c904 > > --- /dev/null > > +++ b/src/spec/models/config_server_spec.rb > > @@ -0,0 +1,42 @@ > > +require 'spec_helper' > > + > > +describe ConfigServer do > > + describe "standard behavior" do > > + before(:each) do > > + @config_server = Factory.build :mock_config_server > > + end > > + > > + it "should require a host" do > > + @config_server.should be_valid > > + @config_server.host = nil > > + @config_server.should_not be_valid > > + end > > + > > + it "should require a port" do > > + @config_server.should be_valid > > + @config_server.port = nil > > + @config_server.should_not be_valid > > + end > > + > > + it "should suggest https when a cert is present" do > > + @config_server.certificate = "abc" > > + @config_server.base_url.should =~ /https:\/\/.*/ > > + end > > + > > + it "should suggest http when a cert is not present" do > > + @config_server.certificate = nil > > + @config_server.base_url.should =~ /http:\/\/.*/ > > + end > > + end > > + > > + describe "error behavior: invalid credentials" do > > + before(:each) do > > + @config_server = Factory.build :invalid_credentials_config_server > > + end > > + > > + it "should report an error when unauthorized" do > > + @config_server.should_not be_valid > > + @config_server.errors.full_messages.join(" ").should include_text("Could not validate config server connection") > > + end > > + end > > +end > > diff --git a/src/spec/models/provider_account_spec.rb b/src/spec/models/provider_account_spec.rb > > index 2c3c090..99f11e3 100644 > > --- a/src/spec/models/provider_account_spec.rb > > +++ b/src/spec/models/provider_account_spec.rb > > @@ -16,6 +16,16 @@ describe ProviderAccount do > > @provider_account.should be_frozen > > end > > This ties to the destroyable? method in the model. I don't think it's a problem to destroy an account that has a config server hung onto it, so I'd remove this code. However, if there is a good reason for that, let me know and it can go in.

> > + it "should not be destroyable if it has config servers" do > > + @provider_account.config_server = ConfigServer.new > > + @provider_account.destroyable?.should be_false > > + @provider_account.destroy.should be_false > > + @provider_account.config_server = nil > > + @provider_account.destroyable?.should be_true > > + @provider_account.destroy.equal?((a)provider_account).should be_true > > + @provider_account.should be_frozen > > + end > > + > > it "should check the validitiy of the cloud account login credentials" do > > mock_provider = Factory :mock_provider > >

On 07/12/2011 10:32 PM, gblomqui(a)redhat.com wrote: > From: Greg Blomquist&lt;gblomqui(a)redhat.com&gt; > > This commit adds the ability to manage config server data as an object > associated to provider accounts. Specifically, provider accounts can have > 0-or-1 config servers. > > Now, a user can add, edit, delete, and test a config server from the provider account > view. > > Signed-off-by: Greg Blomquist&lt;gblomqui(a)redhat.com&gt; > --- > src/app/controllers/config_servers_controller.rb | 72 ++++++++++ > src/app/models/config_server.rb | 147 ++++++++++++++++++++ > src/app/models/provider_account.rb | 2 + > .../views/config_servers/_config_server_form.haml | 22 +++ > src/app/views/config_servers/_section_header.haml | 3 + > src/app/views/config_servers/edit.haml | 21 +++ > src/app/views/config_servers/new.haml | 16 ++ > src/app/views/provider_accounts/_properties.haml | 22 +++ > src/config/locales/en.yml | 14 ++ > src/config/routes.rb | 1 + > .../migrate/20110606141425_create_config_server.rb | 18 +++ > src/features/config_server.feature | 109 +++++++++++++++ > .../step_definitions/config_server_steps.rb | 37 +++++ > src/features/support/env.rb | 3 + > src/features/support/paths.rb | 8 +- > .../controllers/config_servers_controller_spec.rb | 104 ++++++++++++++ > src/spec/factories/config_server.rb | 36 +++++ > src/spec/models/config_server_spec.rb | 42 ++++++ > src/spec/models/provider_account_spec.rb | 7 + > 19 files changed, 683 insertions(+), 1 deletions(-) > create mode 100644 src/app/controllers/config_servers_controller.rb > create mode 100644 src/app/models/config_server.rb > create mode 100644 src/app/views/config_servers/_config_server_form.haml > create mode 100644 src/app/views/config_servers/_section_header.haml > create mode 100644 src/app/views/config_servers/edit.haml > create mode 100644 src/app/views/config_servers/new.haml > create mode 100644 src/db/migrate/20110606141425_create_config_server.rb > create mode 100644 src/features/config_server.feature > create mode 100644 src/features/step_definitions/config_server_steps.rb > create mode 100644 src/spec/controllers/config_servers_controller_spec.rb > create mode 100644 src/spec/factories/config_server.rb > create mode 100644 src/spec/models/config_server_spec.rb > > diff --git a/src/app/controllers/config_servers_controller.rb b/src/app/controllers/config_servers_controller.rb > new file mode 100644 > index 0000000..466f1b7 > --- /dev/null > +++ b/src/app/controllers/config_servers_controller.rb > @@ -0,0 +1,72 @@ > +require 'rest_client' > + > +class ConfigServersController< ApplicationController > + before_filter :require_user > + layout 'application' > + > + def top_section > + :administer > + end > + > + def edit > + @config_server = ConfigServer.find(params[:id]) > + @provider_account = @config_server.provider_account We should check for the permissions here, too. The reason isn't security but user experience. Without the permission check, the user will be able to get to this form, fill stuff in and only upon submit get's the "sorry, can't do that, Dave" message. If we have a permission check here as well, we'll prevent the user from even going there and save their time and effort.

> + end > + > + def new > + @provider_account = ProviderAccount.find(params[:provider_account_id]) > + @config_server = ConfigServer.new() Same here: it's better to put the permission check here as well.

> + end > + > + def test > + config_server = ConfigServer.find(params[:id]) > + if not config_server.connection_valid? > + flash[:error] = config_server.connection_error_msg > + else > + flash[:notice] = "Test successful" > + end > + redirect_to provider_account_path(config_server.provider_account.id) > + end > + > + def create > + @provider_account = ProviderAccount.find(params[:provider_account_id]) > + # for now the privileges required to create, modify, or delete a config > + # server are tied to modifying the particular associated provider account > + require_privilege(Privilege::MODIFY, @provider_account) > + > + @config_server = ConfigServer.new(params[:config_server]) > + @config_server.provider_account = @provider_account > + if @config_server.invalid? > + flash[:error] = "The config server information is invalid." > + render :action => 'new' and return > + end > + @config_server.save! > + flash[:notice] = "Config server added." > + redirect_to provider_account_path(@provider_account) > + end > + > + def update > + @config_server = ConfigServer.find(params[:id]) > + @provider_account = @config_server.provider_account > + require_privilege(Privilege::MODIFY, @provider_account) > + > + if @config_server.update_attributes(params[:config_server]) > + flash[:notice] = "Config server updated." > + redirect_to provider_account_path(@provider_account) > + else > + flash[:error] = "Config server was not updated" > + render :action => :edit > + end > + end > + > + def destroy > + @config_server = ConfigServer.find(params[:id]) > + require_privilege(Privilege::MODIFY, @config_server.provider_account) > + if ConfigServer.destroy(params[:id]) > + flash[:notice] = "Config server was deleted." > + else > + flash[:error] = "Config server was not deleted" > + end > + redirect_to provider_account_path((a)config_server.provider_account) > + end > +end > diff --git a/src/app/models/config_server.rb b/src/app/models/config_server.rb > new file mode 100644 > index 0000000..1582a6d > --- /dev/null > +++ b/src/app/models/config_server.rb > @@ -0,0 +1,147 @@ > +# == Schema Information > +# Schema version: 20110517095823 > +# > +# Table name: config_servers > +# > +# id :integer not null, primary key > +# host :string(255) not null > +# port :string(255) not null > +# username :string(255) null > +# password :string(255) null > +# certificate :string(255) null > +# provider_id :integer not null, fk_provider > +# > + > +## Copyright (C) 2011 Red Hat, Inc. > +## Written by Greg Blomquist&lt;gblomqui(a)redhat.com&gt; > +## > +## This program is free software; you can redistribute it and/or modify > +## it under the terms of the GNU General Public License as published by > +## the Free Software Foundation; version 2 of the License. > +## > +## This program is distributed in the hope that it will be useful, > +## but WITHOUT ANY WARRANTY; without even the implied warranty of > +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +## GNU General Public License for more details. > +## > +## You should have received a copy of the GNU General Public License > +## along with this program; if not, write to the Free Software > +## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, > +## MA 02110-1301, USA. A copy of the GNU General Public License is > +## also available at http://www.gnu.org/copyleft/gpl.html. > +# > + > +class ConfigServer< ActiveRecord::Base > + > + class ConnectionStatus > + attr_reader :state, :message > + def initialize > + @state = :UNTESTED > + end > + > + def untested? > + return :UNTESTED == @state > + end > + > + def fail!(message) > + @state = :FAILURE > + @message = message > + end > + def fail? > + return :FAILURE == @state > + end > + > + def success!(message=nil) > + @state = :SUCCESS > + @message = message > + end > + def success? > + return :SUCCESS == @state > + end > + end > + @@status_fields = [:host, :port, :username, :password, :certificate] > + > + belongs_to :provider_account > + > + validates_presence_of :host > + validates_presence_of :port > + validate :validate_connection > + > + # Utility method to piece together the connection information for this config > + # server. > + def base_url > + scheme = (certificate.blank?) ? "http" : "https" > + domain = host > + if not port.blank? > + domain += ":#{port}" > + end > + "#{scheme}://#{domain}" > + end > + > + # Reports the error message (if any) produced by testing the connection to > + # this config server. > + def connection_error_msg > + if not connection_valid? > + return status.message > + end > + end > + > + # Determines if the connection represented by #host, #port, #username, > + # #password, and #certificate is a valid connection. If the connection is > + # invalid, calling #connection_error_msg will return the error that was > + # generated when testing the connection. > + def connection_valid? > + if status.untested? or (changed? and (changes.keys& @(a)status_fields).empty?) > + begin > + test_connection > + status.success! > + rescue => e > + error_str = map_connection_exception_to_error(e) > + status.fail!(error_str) > + end > + end > + status.success? > + end > + > + private > + def status > + # smells like a factory in here > + @status = ConnectionStatus.new() if @status.nil? > + @status Ruby has a neat trick to make this kind of code shorter. The entire body can be replaced with this: @status ||= ConnectionStatus.new

The || operator short-circuits. If the first argument evaluates as true, it's returned and the rest is not evaluated. Otherwise the second expression is the result. This doesn't return `true` or `false` but the actual values that were passed to the operator. Checking for `nil?` is unnecessary unless you really need to distinguish between `nil` and `false`. And every assignment returns the assigned value. > + end > + > + def validate_connection > + # for validation hook > + if not connection_valid? > + errors.add_to_base(status.message) > + end > + end > + > + # Test the connection to this config server. Return nil on success, or throw > + # an exception on errors. See > + # http://rubydoc.info/gems/rest-client/1.6.3/RestClient#STATUSES-constant > + # for more information on the types of exceptions. > + def test_connection > + args = {:method => :get} > + if not username.blank? > + args[:user] = username > + args[:password] = password > + end > + if not certificate.blank? > + args[:ssl_client_cert] = OpenSSL::X509::Certificate.new(certificate) > + end > + args[:url] = "#{base_url}/version" > + args[:raw_response] = true > + RestClient::Request.execute(args) > + end > + > + def map_connection_exception_to_error(ex) > + if ex.class == OpenSSL::X509::CertificateError > + error_string = I18n.translate("config_servers.errors.connection.certificate", :url => base_url, :msg => ex.message) > + elsif ex.kind_of?(RestClient::ExceptionWithResponse) or ex.kind_of?(RestClient::Exception) or ex.class == Errno::ETIMEDOUT > + error_string = I18n.translate("config_servers.errors.connection.generic_with_message", :url => base_url, :msg => ex.message) > + elsif not ex.nil? > + error_string = I18n.translate("config_servers.errors.connection.generic", :url => base_url) > + end > + end > +end > diff --git a/src/app/models/provider_account.rb b/src/app/models/provider_account.rb > index 9c5f316..08eb32f 100644 > --- a/src/app/models/provider_account.rb > +++ b/src/app/models/provider_account.rb > @@ -57,6 +57,8 @@ class ProviderAccount< ActiveRecord::Base > # validation of credentials is done in provider_account validation, :validate => false prevents nested_attributes from validation > has_many :credentials, :dependent => :destroy, :validate => false > accepts_nested_attributes_for :credentials > + # eventually, this might be "has_many", but first pass is one-to-one > + has_one :config_server, :dependent => :destroy > > # Helpers > attr_accessor :x509_cert_priv_file, :x509_cert_pub_file > diff --git a/src/app/views/config_servers/_config_server_form.haml b/src/app/views/config_servers/_config_server_form.haml > new file mode 100644 > index 0000000..5e60896 > --- /dev/null > +++ b/src/app/views/config_servers/_config_server_form.haml > @@ -0,0 +1,22 @@ > +%fieldset.clearfix > + - if not @config_server.id.nil? > + = hidden_field_tag "config_server[id]", @config_server.id > + = hidden_field_tag "provider_account_id", @provider_account.id > + = label_tag "config_server[host]", "Host: " > + = text_field_tag "config_server[host]", @config_server.host > +%fieldset.clearfix > + = label_tag "config_server[port]", "Port: " > + = text_field_tag "config_server[port]", @config_server.port > +%fieldset.clearfix > + = label_tag "config_server[username]", "Username: " > + = text_field_tag "config_server[username]", @config_server.username > +%fieldset.clearfix > + = label_tag "config_server[password]", "Password: " > + = password_field_tag "config_server[password]", @config_server.password > +%fieldset.clearfix > + = label_tag "config_server[certificate]", "Certificate: " > + = text_area_tag "config_server[certificate]", @config_server.certificate, :rows => 20, :cols => 64 > + = t('config_servers.certificate_help') > +%fieldset.clearfix > + .grid_13.alpha.omega > + = submit_tag t(:save), :class => "ra nomargin dialogbutton" > diff --git a/src/app/views/config_servers/_section_header.haml b/src/app/views/config_servers/_section_header.haml > new file mode 100644 > index 0000000..d8e75fe > --- /dev/null > +++ b/src/app/views/config_servers/_section_header.haml > @@ -0,0 +1,3 @@ > +%header.page-header > + %h1{:class => controller.controller_name} Config Server > + .corner&nbsp; > diff --git a/src/app/views/config_servers/edit.haml b/src/app/views/config_servers/edit.haml > new file mode 100644 > index 0000000..42e40f8 > --- /dev/null > +++ b/src/app/views/config_servers/edit.haml > @@ -0,0 +1,21 @@ > += render :partial => 'layouts/admin_header' > +%header.page-header > + %h1{:class => controller.controller_name}= &quot;#{(a)provider_account.name} Config Server" > + #obj_actions.button-container > + %div.button-group > + = link_to 'Cancel Editing', provider_account_path(@provider_account), :class => 'button pill danger', :id => 'cancel_config_server_edit' > + .corner&nbsp; > + > +%section.content-section.config_server > + %header > + %h2 Edit Config Server > + > + .content > + = error_messages_for :config_server > + %h2 > + = t('config_servers.edit.edit_config_server') > + - form_tag(config_server_path, { :method => :put }) do > + = render :partial => 'config_server_form', :locals => {:from => "edit"} > + -#%fieldset.clearfix > + .grid_13.alpha.omega > + = submit_tag t(:save), :class => "ra nomargin dialogbutton" > diff --git a/src/app/views/config_servers/new.haml b/src/app/views/config_servers/new.haml > new file mode 100644 > index 0000000..c21aee4 > --- /dev/null > +++ b/src/app/views/config_servers/new.haml > @@ -0,0 +1,16 @@ > += render :partial => 'layouts/admin_header' > += render :partial => 'section_header' > + > +%section.content-section.config_server > + %header > + %h2 New Config Server > + > + .content > + = error_messages_for :config_server > + %h2 > + = t('config_servers.new.new_config_server') > + - form_for(@config_server, :url => config_servers_path) do |f| > + = render :partial => 'config_server_form', :locals => {:from => "new"} > + -#%fieldset > + .grid_13.alpha.omega > + = f.submit t(:add), :class => "ra nomargin dialogbutton" > diff --git a/src/app/views/provider_accounts/_properties.haml b/src/app/views/provider_accounts/_properties.haml > index 559bdf1..6990a17 100644 > --- a/src/app/views/provider_accounts/_properties.haml > +++ b/src/app/views/provider_accounts/_properties.haml > @@ -17,3 +17,25 @@ > = label_tag "Account number:" > %td > = @account_id > + %tr > + %td > + %label Config Server: > + %td > + - missing_config_server = @account.config_server.nil? > + %span#config_server > + = missing_config_server ? "None" : @account.config_server.base_url > + %span#config_server_control > + - if missing_config_server > + [ > + = link_to 'Add', new_config_server_url + &quot;?provider_account_id=#{(a)account.id}&quot; > + ] > + - else > + [ > + = link_to 'Edit', edit_config_server_path((a)account.config_server) > + ] > + [ > + = link_to 'Test', test_config_servers_path + &quot;?id=#{(a)account.config_server.id}&quot; > + ] > + [ > + = link_to 'Delete', config_server_path((a)account.config_server), :method => 'delete', :confirm => "Are you sure you want to delete this config server?" > + ] > diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml > index 1564983..9af4ab8 100644 > --- a/src/config/locales/en.yml > +++ b/src/config/locales/en.yml > @@ -237,6 +237,8 @@ en: > account_not_added: > one: "Account %{list} could not be added" > other: "Accounts %{list} could not be added" > + config_server_saved: "Config Server data was successfully saved" > + config_server_not_saved: "An error occurred while saving the Config Server data." > new: > new_provider_account: New Account > required_field: Required field. > @@ -254,4 +256,16 @@ en: > account_number: AWS Account ID > account_private_cert: EC2 x509 private key > account_public_cert: EC2 x509 public key > + config_servers: > + certificate_help: Provide a certificate to enable https support > + new: > + new_config_server: New Config Server > + edit: > + edit_config_server: Edit Config Server > + errors: > + connection: > + generic: "Could not validate config server connection (url: %{url}). Check the connection parameters and try again." > + generic_with_message: "Could not validate config server connection (url: %{url}). %{msg}" > + certificate: "Could not validate config server connection (url: %{url}). Certificate error: %{msg}" > + unhandled_response: "Could not validate config server connection (url: %{url}). HTTP Status Code: %{code}" > uptime: Uptime > diff --git a/src/config/routes.rb b/src/config/routes.rb > index 7f83338..18ae28d 100644 > --- a/src/config/routes.rb > +++ b/src/config/routes.rb > @@ -46,6 +46,7 @@ ActionController::Routing::Routes.draw do |map| > map.resources :provider_types, :only => :index > map.resources :users, :collection => { :multi_destroy => :delete } > map.resources :provider_accounts, :collection => { :multi_destroy => :delete, :set_selected_provider => :get} > + map.resources :config_servers, :collection => { :test => :get } > map.resources :roles, :collection => { :multi_destroy => :delete } > map.resources :settings, :collection => { :self_service => :get, :general_settings => :get } > map.resources :pool_families, :collection => { :multi_destroy => :delete, :add_provider_account => :post, :multi_destroy_provider_accounts => :delete } > diff --git a/src/db/migrate/20110606141425_create_config_server.rb b/src/db/migrate/20110606141425_create_config_server.rb > new file mode 100644 > index 0000000..e10ce78 > --- /dev/null > +++ b/src/db/migrate/20110606141425_create_config_server.rb > @@ -0,0 +1,18 @@ > +class CreateConfigServer< ActiveRecord::Migration > + def self.up > + create_table :config_servers do |t| > + t.string :host, :null => false > + t.string :port, :null => false > + t.string :username, :null => true > + t.string :password, :null => true > + t.string :certificate, :null => true, :limit => 2048 > + t.integer :provider_account_id, :null => false > + > + t.timestamps > + end > + end > + > + def self.down > + drop_table :config_servers > + end > +end > diff --git a/src/features/config_server.feature b/src/features/config_server.feature > new file mode 100644 > index 0000000..9beecf8 > --- /dev/null > +++ b/src/features/config_server.feature > @@ -0,0 +1,109 @@ > +Feature: Config Servers > + In order to administer configuration management on systems > + As a user > + I want to manage config servers as part of provider accounts > + > + Background: > + Given I am an authorised user > + And I am logged in > + > + # This scenario relies on a stubbed version of ConfigServer > + # that never fails on the connection test > + Scenario: I am able to add a config server to a provider account > + Given I am on the homepage > + And there is mock provider account "mock_account" > + And I want to add a new config server > + When I go to mock_account's provider account page > + Then I should see "None" within "#config_server" > + And I should see "[ Add ]" within "#config_server_control" > + When I follow "Add" > + Then I should be on the new config server page > + When I fill in "config_server[host]" with "valid_host" > + When I fill in "config_server[port]" with "valid_port" > + And I press "Save" > + Then I should be on mock_account's provider account page > + And I should see "Config server added" > + And I should see "[ Edit ]" within "#config_server_control" > + > + # This is essentially the same scenario as the first, but creates > + # a different stubbed ConfigServer, so it fails > + Scenario: I cannot add a config server with invalid host or port information > + Given I am on the homepage > + And there is mock provider account "mock_account" > + And I am not sure about the config server host > + When I go to mock_account's provider account page > + Then I should see "None" within "#config_server" > + And I should see "[ Add ]" within "#config_server_control" > + When I follow "Add" > + Then I should be on the new config server page > + When I fill in "config_server[host]" with "invalid" > + When I fill in "config_server[port]" with "invalid" > + And I press "Save" > + Then I should see "The config server information is invalid" > + And I should see "Could not validate config server connection" > + > + # This is essentially the same scenario as the first, but creates > + # a different stubbed ConfigServer, so it fails > + Scenario: I cannot add a config server with invalid credentials > + Given I am on the homepage > + And there is mock provider account "mock_account" > + And I am not sure about the config server credentials > + When I go to mock_account's provider account page > + Then I should see "None" within "#config_server" > + And I should see "[ Add ]" within "#config_server_control" > + When I follow "Add" > + Then I should be on the new config server page > + When I fill in "config_server[host]" with "valid" > + When I fill in "config_server[port]" with "valid" > + When I fill in "config_server[username]" with "invalid" > + When I fill in "config_server[password]" with "invalid" > + And I press "Save" > + Then I should see "The config server information is invalid" > + And I should see "Could not validate config server connection" > + > + Scenario: I should be able to edit a config server > + Given I am on the homepage > + And there is a mock config server "https://mock:443" for account "mock_account" > + When I go to mock_account's provider account page > + Then I should see "[ Edit ]" within "#config_server_control" > + When I follow "Edit" within "#config_server_control" > + Then I should be on the edit config server page for account "mock_account" > + And I press "Save" > + Then I should be on mock_account's provider account page > + And I should see "Config server updated" > + > + Scenario: I should be able to delete an existing config server > + Given I am on the homepage > + And there is a mock config server "https://mock:443" for account "mock_account" > + When I go to mock_account's provider account page > + Then I should see "[ Delete ]" within "#config_server_control" > + When I follow "Delete" within "#config_server_control" > + Then I should see "Config server was deleted" > + And I should be on mock_account's provider account page > + > + Scenario: I should be able to test a correctly configured and available config server > + Given I am on the homepage > + And there is a mock config server "https://mock:443" for account "mock_account" > + When I go to mock_account's provider account page > + Then I should see "[ Test ]" within "#config_server_control" > + When I follow "Test" within "#config_server_control" > + Then I should see "Test successful" > + And I should be on mock_account's provider account page > + > + Scenario: I should see an error when I test a config server with invalid credentials > + Given I am on the homepage > + And there is a mock config server "https://bad_credentials:443" for account "mock_account" > + When I go to mock_account's provider account page > + Then I should see "[ Test ]" within "#config_server_control" > + When I follow "Test" within "#config_server_control" > + Then I should see "Could not validate config server connection" > + And I should see "Unauthorized" > + > + Scenario: I should see an error when I test a config server with an invalid host > + Given I am on the homepage > + And there is a mock config server "https://bad_host:443" for account "mock_account" > + When I go to mock_account's provider account page > + Then I should see "[ Test ]" within "#config_server_control" > + When I follow "Test" within "#config_server_control" > + Then I should see "Could not validate config server connection" > + And I should see "Connection timed out" > diff --git a/src/features/step_definitions/config_server_steps.rb b/src/features/step_definitions/config_server_steps.rb > new file mode 100644 > index 0000000..438ad00 > --- /dev/null > +++ b/src/features/step_definitions/config_server_steps.rb > @@ -0,0 +1,37 @@ > +Given /^I want to add a new config server$/ do > + c = Factory.build(:mock_config_server) > + ConfigServer.stub!(:new).and_return(c) > +end > + > +Given /^I am not sure about the config server (host|port)$/ do |host_or_port| > + c = Factory.build(:invalid_host_or_port_config_server) > + ConfigServer.stub!(:new).and_return(c) > +end > + > +Given /^I am not sure about the config server credentials$/ do > + c = Factory.build(:invalid_credentials_config_server) > + ConfigServer.stub!(:new).and_return(c) > +end > + > +Given /^there is a mock config server "(http|https):\/\/(.*):(.*)" for account "(.*)"$/ do |scheme,host,port,acc| > + provider = Factory :mock_provider, :name => "mock_provider" > + mock_account = Factory :mock_provider_account, :label => acc, :provider => provider > + params = {:host => host, :port => port, :provider_account => mock_account} > + if "https" == scheme > + params[:certificate] = "cert" > + end > + @config_server = Factory :mock_config_server, params > + # Don't particularly like this next bit, but the problem is the "create" > + # default_strategy used when instantiating the Factory leaves the > + # @config_server in a "SUCCESS" status. This bit effectively "resets" the > + # status to "UNTESTED" so the :test_connection method(stub) has to get called > + @config_server.stub!(:status).and_return(ConfigServer::ConnectionStatus.new()) > + case host > + when "bad_credentials" > + @config_server.stub!(:test_connection).and_raise(RestClient::Unauthorized) > + when "bad_host" > + @config_server.stub!(:test_connection).and_raise(Errno::ETIMEDOUT) > + end > + # ensure that the :mock_config_server (with stubbed methods) is returned > + ConfigServer.stub!(:find).and_return(@config_server) > +end > diff --git a/src/features/support/env.rb b/src/features/support/env.rb > index 33f9ed2..7357eda 100644 > --- a/src/features/support/env.rb > +++ b/src/features/support/env.rb > @@ -19,6 +19,9 @@ require File.expand_path(File.dirname(__FILE__) + '../../../spec/vcr_setup.rb') > require 'webrat' > require 'webrat/core/matchers' > > +# Pull in the ability to stub out methods > +require 'spec/stubs/cucumber' > + > Webrat.configure do |config| > config.mode = :rails > config.open_error_files = false # Set to true if you want error pages to pop up in the browser > diff --git a/src/features/support/paths.rb b/src/features/support/paths.rb > index 12a72d5..3a86c52 100644 > --- a/src/features/support/paths.rb > +++ b/src/features/support/paths.rb > @@ -98,6 +98,13 @@ module NavigationHelpers > when /^(.*)'s provider account page$/ > provider_account_path(ProviderAccount.find_by_label($1)) > > + when /the new config server page/ > + url_for new_config_server_path > + > + when /^the edit config server page/ > + @config_server.stub!(:connection_valid?).and_return(true) > + edit_config_server_path(@config_server) > + > when /the operational status of deployment page/ > deployment_path(@deployment, :details_tab => 'operation') > > @@ -123,7 +130,6 @@ module NavigationHelpers > when /^the (.*)'s edit user page$/ > edit_user_path(User.find_by_login($1)) > > - > # Add more mappings here. > # Here is an example that pulls values out of the Regexp: > # > diff --git a/src/spec/controllers/config_servers_controller_spec.rb b/src/spec/controllers/config_servers_controller_spec.rb > new file mode 100644 > index 0000000..9b2f1a0 > --- /dev/null > +++ b/src/spec/controllers/config_servers_controller_spec.rb > @@ -0,0 +1,104 @@ > +require 'spec_helper' > + > +describe ConfigServersController do > + fixtures :all > + > + before(:each) do > + @admin_permission = Factory :admin_permission > + @admin = @admin_permission.user > + activate_authlogic > + @session = UserSession.create(@admin) > + end > + > + context "editing config servers" do > + before(:each) do > + @config_server = Factory :mock_config_server > + @provider_account = @config_server.provider_account > + end > + > + it "should provide UI to edit an existing Config Server" do > + get :edit, :id => @config_server.id > + response.should be_success > + response.should render_template("edit") > + end > + > + it "should allow users with account modify permissions to update a Config Server" do > + post :update, :id => @config_server.id, :config_server => {:host => "host", :port => "port"} > + response.should be_success > + end > + end > + > + context "creating config servers" do > + before(:each) do > + @provider_account = Factory :mock_provider_account > + end > + > + it "should provide UI to create a new Config Server" do > + get :new, :provider_account_id => @provider_account.id > + response.should be_success > + response.should render_template("new") > + end > + > + it "should allow users with account modify permissions to create a Config Server" do > + config_server = Factory :mock_config_server, :host => "host", :port => "port" > + ConfigServer.stub!(:new).and_return(config_server) > + post :create, :provider_account_id => @provider_account.id, > + :config_server => { > + :host => "host", > + :port => "port" > + } > + response.should redirect_to(provider_account_path((a)provider_account.id)) > + response.flash[:error].should be_nil > + end > + > + it "should fail creating a config server when the username or password is invalid" do > + config_server = Factory :invalid_credentials_config_server, :host => "host", :port => "port", :username => "invalid", :password => "invalid" > + ConfigServer.stub!(:new).and_return(config_server) > + post :create, :provider_account_id => @provider_account.id, > + :config_server => { > + :host => "host", > + :port => "port", > + :username => "invalid", > + :password => "invalid" > + } > + response.should be_success > + response.should render_template("new") > + response.flash[:error].should == "The config server information is invalid." > + end > + > + it "should fail creating a config server when the host and port are invalid" do > + config_server = Factory :invalid_host_or_port_config_server, :host => "invalid", :port => "invalid" > + ConfigServer.stub!(:new).and_return(config_server) > + post :create, :provider_account_id => @provider_account.id, > + :config_server => { > + :host => "invalid", > + :port => "invalid" > + } > + response.should be_success > + response.should render_template("new") > + response.flash[:error].should == "The config server information is invalid." > + end > + > + it "should require that port is provided" do > + post :create, :provider_account_id => @provider_account.id, > + :config_server => { > + :host => "host", > + :port => "" > + } > + response.should be_success > + response.should render_template("new") > + response.flash[:error].should == "The config server information is invalid." > + end > + > + it "should require that host is provided" do > + post :create, :provider_account_id => @provider_account.id, > + :config_server => { > + :host => "", > + :port => "port" > + } > + response.should be_success > + response.should render_template("new") > + response.flash[:error].should == "The config server information is invalid." > + end > + end > +end > diff --git a/src/spec/factories/config_server.rb b/src/spec/factories/config_server.rb > new file mode 100644 > index 0000000..91e07f0 > --- /dev/null > +++ b/src/spec/factories/config_server.rb > @@ -0,0 +1,36 @@ > +Factory.define :config_server do |f| > + f.sequence(:host) {|n| "config_server#{n}" } > +end > + > +Factory.define :base_config_server, :parent => :config_server do |f| > + f.host "localhost" > + f.port "80" > + f.username "username" > + f.password "password" > +end > + > +Factory.define :mock_config_server, :parent => :base_config_server do |f| > + f.association :provider_account, :factory => :mock_provider_account > + f.after_build do |cs| > + cs.stub!(:test_connection).and_return(nil) if cs.respond_to? :stub! > + end > +end > + > +Factory.define :invalid_credentials_config_server, :parent => :base_config_server, :default_strategy => :build do |f| > + f.port "443" > + f.username "bad_username" > + f.password "bad_password" > + f.certificate "cert" > + f.after_build do |cs| > + cs.stub!(:test_connection).and_raise(RestClient::Unauthorized) if cs.respond_to? :stub! > + end > +end > + > +Factory.define :invalid_host_or_port_config_server, :parent => :base_config_server, :default_strategy => :build do |f| > + f.host "bad_host" > + f.port "443" > + f.certificate "cert" > + f.after_build do |cs| > + cs.stub!(:test_connection).and_raise(Errno::ETIMEDOUT) if cs.respond_to? :stub! > + end > +end > diff --git a/src/spec/models/config_server_spec.rb b/src/spec/models/config_server_spec.rb > new file mode 100644 > index 0000000..350f036 > --- /dev/null > +++ b/src/spec/models/config_server_spec.rb > @@ -0,0 +1,42 @@ > +require 'spec_helper' > + > +describe ConfigServer do > + describe "standard behavior" do > + before(:each) do > + @config_server = Factory.build :mock_config_server > + end > + > + it "should require a host" do > + @config_server.should be_valid > + @config_server.host = nil > + @config_server.should_not be_valid > + end > + > + it "should require a port" do > + @config_server.should be_valid > + @config_server.port = nil > + @config_server.should_not be_valid > + end > + > + it "should suggest https when a cert is present" do > + @config_server.certificate = "abc" > + @config_server.base_url.should =~ /https:\/\/.*/ > + end > + > + it "should suggest http when a cert is not present" do > + @config_server.certificate = nil > + @config_server.base_url.should =~ /http:\/\/.*/ > + end > + end > + > + describe "error behavior: invalid credentials" do > + before(:each) do > + @config_server = Factory.build :invalid_credentials_config_server > + end > + > + it "should report an error when unauthorized" do > + @config_server.should_not be_valid > + @config_server.errors.full_messages.join(" ").should include_text("Could not validate config server connection") > + end > + end > +end > diff --git a/src/spec/models/provider_account_spec.rb b/src/spec/models/provider_account_spec.rb > index 02666f1..1d65951 100644 > --- a/src/spec/models/provider_account_spec.rb > +++ b/src/spec/models/provider_account_spec.rb > @@ -16,6 +16,13 @@ describe ProviderAccount do > @provider_account.should be_frozen > end > > + it "should be destroyable if it has a config server" do > + @provider_account.config_server = ConfigServer.new > + @provider_account.destroyable?.should be_true > + @provider_account.destroy.equal?((a)provider_account).should be_true > + @provider_account.should be_frozen > + end > + > it "should check the validitiy of the cloud account login credentials" do > mock_provider = Factory :mock_provider > Hey Greg, Looking good, ACK. I put a few comments inline to make it even more ruby idiomatic; feel free to ignore them and push anyway if you don't like them.

Thomas

On 07/14/2011 05:14 PM, gblomqui(a)redhat.com wrote: > From: Greg Blomquist&lt;gblomqui(a)redhat.com&gt; > > This commit adds the ability to manage config server data as an object > associated to provider accounts. Specifically, provider accounts can have > 0-or-1 config servers. > > Now, a user can add, edit, delete, and test a config server from the provider account > view. > > Signed-off-by: Greg Blomquist&lt;gblomqui(a)redhat.com&gt; > --- > src/app/controllers/config_servers_controller.rb | 74 ++++++++++ > src/app/models/config_server.rb | 146 ++++++++++++++++++++ > src/app/models/provider_account.rb | 2 + > .../views/config_servers/_config_server_form.haml | 22 +++ > src/app/views/config_servers/_section_header.haml | 3 + > src/app/views/config_servers/edit.haml | 21 +++ > src/app/views/config_servers/new.haml | 16 ++ > src/app/views/provider_accounts/_properties.haml | 22 +++ > src/config/locales/en.yml | 14 ++ > src/config/routes.rb | 1 + > .../migrate/20110606141425_create_config_server.rb | 18 +++ > src/features/config_server.feature | 109 +++++++++++++++ > .../step_definitions/config_server_steps.rb | 37 +++++ > src/features/support/env.rb | 3 + > src/features/support/paths.rb | 8 +- > .../controllers/config_servers_controller_spec.rb | 104 ++++++++++++++ > src/spec/factories/config_server.rb | 36 +++++ > src/spec/models/config_server_spec.rb | 42 ++++++ > src/spec/models/provider_account_spec.rb | 7 + > 19 files changed, 684 insertions(+), 1 deletions(-) > create mode 100644 src/app/controllers/config_servers_controller.rb > create mode 100644 src/app/models/config_server.rb > create mode 100644 src/app/views/config_servers/_config_server_form.haml > create mode 100644 src/app/views/config_servers/_section_header.haml > create mode 100644 src/app/views/config_servers/edit.haml > create mode 100644 src/app/views/config_servers/new.haml > create mode 100644 src/db/migrate/20110606141425_create_config_server.rb > create mode 100644 src/features/config_server.feature > create mode 100644 src/features/step_definitions/config_server_steps.rb > create mode 100644 src/spec/controllers/config_servers_controller_spec.rb > create mode 100644 src/spec/factories/config_server.rb > create mode 100644 src/spec/models/config_server_spec.rb > > diff --git a/src/app/controllers/config_servers_controller.rb b/src/app/controllers/config_servers_controller.rb > new file mode 100644 > index 0000000..dd53be1 > --- /dev/null > +++ b/src/app/controllers/config_servers_controller.rb > @@ -0,0 +1,74 @@ > +require 'rest_client' > + > +class ConfigServersController< ApplicationController > + before_filter :require_user > + layout 'application' > + > + def top_section > + :administer > + end > + > + def edit > + @config_server = ConfigServer.find(params[:id]) > + @provider_account = @config_server.provider_account > + require_privilege(Privilege::MODIFY, @provider_account) > + end > + > + def new > + @provider_account = ProviderAccount.find(params[:provider_account_id]) > + require_privilege(Privilege::MODIFY, @provider_account) > + @config_server = ConfigServer.new() > + end > + > + def test > + config_server = ConfigServer.find(params[:id]) > + if not config_server.connection_valid? > + flash[:error] = config_server.connection_error_msg > + else > + flash[:notice] = "Test successful" > + end > + redirect_to provider_account_path(config_server.provider_account.id) > + end > + > + def create > + @provider_account = ProviderAccount.find(params[:provider_account_id]) > + # for now the privileges required to create, modify, or delete a config > + # server are tied to modifying the particular associated provider account > + require_privilege(Privilege::MODIFY, @provider_account) > + > + @config_server = ConfigServer.new(params[:config_server]) > + @config_server.provider_account = @provider_account > + if @config_server.invalid? > + flash[:error] = "The config server information is invalid." > + render :action => 'new' and return > + end > + @config_server.save! > + flash[:notice] = "Config server added." > + redirect_to provider_account_path(@provider_account) > + end > + > + def update > + @config_server = ConfigServer.find(params[:id]) > + @provider_account = @config_server.provider_account > + require_privilege(Privilege::MODIFY, @provider_account) > + > + if @config_server.update_attributes(params[:config_server]) > + flash[:notice] = "Config server updated." > + redirect_to provider_account_path(@provider_account) > + else > + flash[:error] = "Config server was not updated" > + render :action => :edit > + end > + end > + > + def destroy > + @config_server = ConfigServer.find(params[:id]) > + require_privilege(Privilege::MODIFY, @config_server.provider_account) > + if ConfigServer.destroy(params[:id]) > + flash[:notice] = "Config server was deleted." > + else > + flash[:error] = "Config server was not deleted" > + end > + redirect_to provider_account_path((a)config_server.provider_account) > + end > +end > diff --git a/src/app/models/config_server.rb b/src/app/models/config_server.rb > new file mode 100644 > index 0000000..9ccba5f > --- /dev/null > +++ b/src/app/models/config_server.rb > @@ -0,0 +1,146 @@ > +# == Schema Information > +# Schema version: 20110517095823 > +# > +# Table name: config_servers > +# > +# id :integer not null, primary key > +# host :string(255) not null > +# port :string(255) not null > +# username :string(255) null > +# password :string(255) null > +# certificate :string(255) null > +# provider_id :integer not null, fk_provider > +# > + > +## Copyright (C) 2011 Red Hat, Inc. > +## Written by Greg Blomquist&lt;gblomqui(a)redhat.com&gt; > +## > +## This program is free software; you can redistribute it and/or modify > +## it under the terms of the GNU General Public License as published by > +## the Free Software Foundation; version 2 of the License. > +## > +## This program is distributed in the hope that it will be useful, > +## but WITHOUT ANY WARRANTY; without even the implied warranty of > +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +## GNU General Public License for more details. > +## > +## You should have received a copy of the GNU General Public License > +## along with this program; if not, write to the Free Software > +## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, > +## MA 02110-1301, USA. A copy of the GNU General Public License is > +## also available at http://www.gnu.org/copyleft/gpl.html. > +# > + > +class ConfigServer< ActiveRecord::Base > + > + class ConnectionStatus > + attr_reader :state, :message > + def initialize > + @state = :UNTESTED > + end > + > + def untested? > + return :UNTESTED == @state > + end > + > + def fail!(message) > + @state = :FAILURE > + @message = message > + end > + def fail? > + return :FAILURE == @state > + end > + > + def success!(message=nil) > + @state = :SUCCESS > + @message = message > + end > + def success? > + return :SUCCESS == @state > + end > + end > + @@status_fields = [:host, :port, :username, :password, :certificate] > + > + belongs_to :provider_account > + > + validates_presence_of :host > + validates_presence_of :port > + validate :validate_connection > + > + # Utility method to piece together the connection information for this config > + # server. > + def base_url > + scheme = (certificate.blank?) ? "http" : "https" > + domain = host > + if not port.blank? > + domain += ":#{port}" > + end > + "#{scheme}://#{domain}" > + end > + > + # Reports the error message (if any) produced by testing the connection to > + # this config server. > + def connection_error_msg > + if not connection_valid? > + return status.message > + end > + end > + > + # Determines if the connection represented by #host, #port, #username, > + # #password, and #certificate is a valid connection. If the connection is > + # invalid, calling #connection_error_msg will return the error that was > + # generated when testing the connection. > + def connection_valid? > + if status.untested? or (changed? and (changes.keys& @(a)status_fields).empty?) > + begin > + test_connection > + status.success! > + rescue => e > + error_str = map_connection_exception_to_error(e) > + status.fail!(error_str) > + end > + end > + status.success? > + end > + > + private > + def status > + # smells like a factory in here > + @status ||= ConnectionStatus.new > + end > + > + def validate_connection > + # for validation hook > + if not connection_valid? > + errors.add_to_base(status.message) > + end > + end > + > + # Test the connection to this config server. Return nil on success, or throw > + # an exception on errors. See > + # http://rubydoc.info/gems/rest-client/1.6.3/RestClient#STATUSES-constant > + # for more information on the types of exceptions. > + def test_connection > + args = {:method => :get} > + if not username.blank? > + args[:user] = username > + args[:password] = password > + end > + if not certificate.blank? > + args[:ssl_client_cert] = OpenSSL::X509::Certificate.new(certificate) > + end > + args[:url] = "#{base_url}/version" > + args[:raw_response] = true > + RestClient::Request.execute(args) > + end > + > + def map_connection_exception_to_error(ex) > + if ex.class == OpenSSL::X509::CertificateError > + error_string = I18n.translate("config_servers.errors.connection.certificate", :url => base_url, :msg => ex.message) > + elsif ex.kind_of?(RestClient::ExceptionWithResponse) or ex.kind_of?(RestClient::Exception) or ex.class == Errno::ETIMEDOUT > + error_string = I18n.translate("config_servers.errors.connection.generic_with_message", :url => base_url, :msg => ex.message) > + elsif not ex.nil? > + error_string = I18n.translate("config_servers.errors.connection.generic", :url => base_url) > + end > + end > +end > diff --git a/src/app/models/provider_account.rb b/src/app/models/provider_account.rb > index 812d5a1..3cd3691 100644 > --- a/src/app/models/provider_account.rb > +++ b/src/app/models/provider_account.rb > @@ -51,6 +51,8 @@ class ProviderAccount< ActiveRecord::Base > # validation of credentials is done in provider_account validation, :validate => false prevents nested_attributes from validation > has_many :credentials, :dependent => :destroy, :validate => false > accepts_nested_attributes_for :credentials > + # eventually, this might be "has_many", but first pass is one-to-one > + has_one :config_server, :dependent => :destroy > > # Helpers > attr_accessor :x509_cert_priv_file, :x509_cert_pub_file > diff --git a/src/app/views/config_servers/_config_server_form.haml b/src/app/views/config_servers/_config_server_form.haml > new file mode 100644 > index 0000000..5e60896 > --- /dev/null > +++ b/src/app/views/config_servers/_config_server_form.haml > @@ -0,0 +1,22 @@ > +%fieldset.clearfix > + - if not @config_server.id.nil? > + = hidden_field_tag "config_server[id]", @config_server.id > + = hidden_field_tag "provider_account_id", @provider_account.id > + = label_tag "config_server[host]", "Host: " > + = text_field_tag "config_server[host]", @config_server.host > +%fieldset.clearfix > + = label_tag "config_server[port]", "Port: " > + = text_field_tag "config_server[port]", @config_server.port > +%fieldset.clearfix > + = label_tag "config_server[username]", "Username: " > + = text_field_tag "config_server[username]", @config_server.username > +%fieldset.clearfix > + = label_tag "config_server[password]", "Password: " > + = password_field_tag "config_server[password]", @config_server.password > +%fieldset.clearfix > + = label_tag "config_server[certificate]", "Certificate: " > + = text_area_tag "config_server[certificate]", @config_server.certificate, :rows => 20, :cols => 64 > + = t('config_servers.certificate_help') > +%fieldset.clearfix > + .grid_13.alpha.omega > + = submit_tag t(:save), :class => "ra nomargin dialogbutton" > diff --git a/src/app/views/config_servers/_section_header.haml b/src/app/views/config_servers/_section_header.haml > new file mode 100644 > index 0000000..d8e75fe > --- /dev/null > +++ b/src/app/views/config_servers/_section_header.haml > @@ -0,0 +1,3 @@ > +%header.page-header > + %h1{:class => controller.controller_name} Config Server > + .corner&nbsp; > diff --git a/src/app/views/config_servers/edit.haml b/src/app/views/config_servers/edit.haml > new file mode 100644 > index 0000000..42e40f8 > --- /dev/null > +++ b/src/app/views/config_servers/edit.haml > @@ -0,0 +1,21 @@ > += render :partial => 'layouts/admin_header' > +%header.page-header > + %h1{:class => controller.controller_name}= &quot;#{(a)provider_account.name} Config Server" > + #obj_actions.button-container > + %div.button-group > + = link_to 'Cancel Editing', provider_account_path(@provider_account), :class => 'button pill danger', :id => 'cancel_config_server_edit' > + .corner&nbsp; > + > +%section.content-section.config_server > + %header > + %h2 Edit Config Server > + > + .content > + = error_messages_for :config_server > + %h2 > + = t('config_servers.edit.edit_config_server') > + - form_tag(config_server_path, { :method => :put }) do > + = render :partial => 'config_server_form', :locals => {:from => "edit"} > + -#%fieldset.clearfix > + .grid_13.alpha.omega > + = submit_tag t(:save), :class => "ra nomargin dialogbutton" > diff --git a/src/app/views/config_servers/new.haml b/src/app/views/config_servers/new.haml > new file mode 100644 > index 0000000..c21aee4 > --- /dev/null > +++ b/src/app/views/config_servers/new.haml > @@ -0,0 +1,16 @@ > += render :partial => 'layouts/admin_header' > += render :partial => 'section_header' > + > +%section.content-section.config_server > + %header > + %h2 New Config Server > + > + .content > + = error_messages_for :config_server > + %h2 > + = t('config_servers.new.new_config_server') > + - form_for(@config_server, :url => config_servers_path) do |f| > + = render :partial => 'config_server_form', :locals => {:from => "new"} > + -#%fieldset > + .grid_13.alpha.omega > + = f.submit t(:add), :class => "ra nomargin dialogbutton" > diff --git a/src/app/views/provider_accounts/_properties.haml b/src/app/views/provider_accounts/_properties.haml > index 559bdf1..6990a17 100644 > --- a/src/app/views/provider_accounts/_properties.haml > +++ b/src/app/views/provider_accounts/_properties.haml > @@ -17,3 +17,25 @@ > = label_tag "Account number:" > %td > = @account_id > + %tr > + %td > + %label Config Server: > + %td > + - missing_config_server = @account.config_server.nil? > + %span#config_server > + = missing_config_server ? "None" : @account.config_server.base_url > + %span#config_server_control > + - if missing_config_server > + [ > + = link_to 'Add', new_config_server_url + &quot;?provider_account_id=#{(a)account.id}&quot; > + ] > + - else > + [ > + = link_to 'Edit', edit_config_server_path((a)account.config_server) > + ] > + [ > + = link_to 'Test', test_config_servers_path + &quot;?id=#{(a)account.config_server.id}&quot; > + ] > + [ > + = link_to 'Delete', config_server_path((a)account.config_server), :method => 'delete', :confirm => "Are you sure you want to delete this config server?" > + ] > diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml > index 1564983..9af4ab8 100644 > --- a/src/config/locales/en.yml > +++ b/src/config/locales/en.yml > @@ -237,6 +237,8 @@ en: > account_not_added: > one: "Account %{list} could not be added" > other: "Accounts %{list} could not be added" > + config_server_saved: "Config Server data was successfully saved" > + config_server_not_saved: "An error occurred while saving the Config Server data." > new: > new_provider_account: New Account > required_field: Required field. > @@ -254,4 +256,16 @@ en: > account_number: AWS Account ID > account_private_cert: EC2 x509 private key > account_public_cert: EC2 x509 public key > + config_servers: > + certificate_help: Provide a certificate to enable https support > + new: > + new_config_server: New Config Server > + edit: > + edit_config_server: Edit Config Server > + errors: > + connection: > + generic: "Could not validate config server connection (url: %{url}). Check the connection parameters and try again." > + generic_with_message: "Could not validate config server connection (url: %{url}). %{msg}" > + certificate: "Could not validate config server connection (url: %{url}). Certificate error: %{msg}" > + unhandled_response: "Could not validate config server connection (url: %{url}). HTTP Status Code: %{code}" > uptime: Uptime > diff --git a/src/config/routes.rb b/src/config/routes.rb > index 7f83338..18ae28d 100644 > --- a/src/config/routes.rb > +++ b/src/config/routes.rb > @@ -46,6 +46,7 @@ ActionController::Routing::Routes.draw do |map| > map.resources :provider_types, :only => :index > map.resources :users, :collection => { :multi_destroy => :delete } > map.resources :provider_accounts, :collection => { :multi_destroy => :delete, :set_selected_provider => :get} > + map.resources :config_servers, :collection => { :test => :get } > map.resources :roles, :collection => { :multi_destroy => :delete } > map.resources :settings, :collection => { :self_service => :get, :general_settings => :get } > map.resources :pool_families, :collection => { :multi_destroy => :delete, :add_provider_account => :post, :multi_destroy_provider_accounts => :delete } > diff --git a/src/db/migrate/20110606141425_create_config_server.rb b/src/db/migrate/20110606141425_create_config_server.rb > new file mode 100644 > index 0000000..e10ce78 > --- /dev/null > +++ b/src/db/migrate/20110606141425_create_config_server.rb > @@ -0,0 +1,18 @@ > +class CreateConfigServer< ActiveRecord::Migration > + def self.up > + create_table :config_servers do |t| > + t.string :host, :null => false > + t.string :port, :null => false > + t.string :username, :null => true > + t.string :password, :null => true > + t.string :certificate, :null => true, :limit => 2048 > + t.integer :provider_account_id, :null => false > + > + t.timestamps > + end > + end > + > + def self.down > + drop_table :config_servers > + end > +end > diff --git a/src/features/config_server.feature b/src/features/config_server.feature > new file mode 100644 > index 0000000..9beecf8 > --- /dev/null > +++ b/src/features/config_server.feature > @@ -0,0 +1,109 @@ > +Feature: Config Servers > + In order to administer configuration management on systems > + As a user > + I want to manage config servers as part of provider accounts > + > + Background: > + Given I am an authorised user > + And I am logged in > + > + # This scenario relies on a stubbed version of ConfigServer > + # that never fails on the connection test > + Scenario: I am able to add a config server to a provider account > + Given I am on the homepage > + And there is mock provider account "mock_account" > + And I want to add a new config server > + When I go to mock_account's provider account page > + Then I should see "None" within "#config_server" > + And I should see "[ Add ]" within "#config_server_control" > + When I follow "Add" > + Then I should be on the new config server page > + When I fill in "config_server[host]" with "valid_host" > + When I fill in "config_server[port]" with "valid_port" > + And I press "Save" > + Then I should be on mock_account's provider account page > + And I should see "Config server added" > + And I should see "[ Edit ]" within "#config_server_control" > + > + # This is essentially the same scenario as the first, but creates > + # a different stubbed ConfigServer, so it fails > + Scenario: I cannot add a config server with invalid host or port information > + Given I am on the homepage > + And there is mock provider account "mock_account" > + And I am not sure about the config server host > + When I go to mock_account's provider account page > + Then I should see "None" within "#config_server" > + And I should see "[ Add ]" within "#config_server_control" > + When I follow "Add" > + Then I should be on the new config server page > + When I fill in "config_server[host]" with "invalid" > + When I fill in "config_server[port]" with "invalid" > + And I press "Save" > + Then I should see "The config server information is invalid" > + And I should see "Could not validate config server connection" > + > + # This is essentially the same scenario as the first, but creates > + # a different stubbed ConfigServer, so it fails > + Scenario: I cannot add a config server with invalid credentials > + Given I am on the homepage > + And there is mock provider account "mock_account" > + And I am not sure about the config server credentials > + When I go to mock_account's provider account page > + Then I should see "None" within "#config_server" > + And I should see "[ Add ]" within "#config_server_control" > + When I follow "Add" > + Then I should be on the new config server page > + When I fill in "config_server[host]" with "valid" > + When I fill in "config_server[port]" with "valid" > + When I fill in "config_server[username]" with "invalid" > + When I fill in "config_server[password]" with "invalid" > + And I press "Save" > + Then I should see "The config server information is invalid" > + And I should see "Could not validate config server connection" > + > + Scenario: I should be able to edit a config server > + Given I am on the homepage > + And there is a mock config server "https://mock:443" for account "mock_account" > + When I go to mock_account's provider account page > + Then I should see "[ Edit ]" within "#config_server_control" > + When I follow "Edit" within "#config_server_control" > + Then I should be on the edit config server page for account "mock_account" > + And I press "Save" > + Then I should be on mock_account's provider account page > + And I should see "Config server updated" > + > + Scenario: I should be able to delete an existing config server > + Given I am on the homepage > + And there is a mock config server "https://mock:443" for account "mock_account" > + When I go to mock_account's provider account page > + Then I should see "[ Delete ]" within "#config_server_control" > + When I follow "Delete" within "#config_server_control" > + Then I should see "Config server was deleted" > + And I should be on mock_account's provider account page > + > + Scenario: I should be able to test a correctly configured and available config server > + Given I am on the homepage > + And there is a mock config server "https://mock:443" for account "mock_account" > + When I go to mock_account's provider account page > + Then I should see "[ Test ]" within "#config_server_control" > + When I follow "Test" within "#config_server_control" > + Then I should see "Test successful" > + And I should be on mock_account's provider account page > + > + Scenario: I should see an error when I test a config server with invalid credentials > + Given I am on the homepage > + And there is a mock config server "https://bad_credentials:443" for account "mock_account" > + When I go to mock_account's provider account page > + Then I should see "[ Test ]" within "#config_server_control" > + When I follow "Test" within "#config_server_control" > + Then I should see "Could not validate config server connection" > + And I should see "Unauthorized" > + > + Scenario: I should see an error when I test a config server with an invalid host > + Given I am on the homepage > + And there is a mock config server "https://bad_host:443" for account "mock_account" > + When I go to mock_account's provider account page > + Then I should see "[ Test ]" within "#config_server_control" > + When I follow "Test" within "#config_server_control" > + Then I should see "Could not validate config server connection" > + And I should see "Connection timed out" > diff --git a/src/features/step_definitions/config_server_steps.rb b/src/features/step_definitions/config_server_steps.rb > new file mode 100644 > index 0000000..438ad00 > --- /dev/null > +++ b/src/features/step_definitions/config_server_steps.rb > @@ -0,0 +1,37 @@ > +Given /^I want to add a new config server$/ do > + c = Factory.build(:mock_config_server) > + ConfigServer.stub!(:new).and_return(c) > +end > + > +Given /^I am not sure about the config server (host|port)$/ do |host_or_port| > + c = Factory.build(:invalid_host_or_port_config_server) > + ConfigServer.stub!(:new).and_return(c) > +end > + > +Given /^I am not sure about the config server credentials$/ do > + c = Factory.build(:invalid_credentials_config_server) > + ConfigServer.stub!(:new).and_return(c) > +end > + > +Given /^there is a mock config server "(http|https):\/\/(.*):(.*)" for account "(.*)"$/ do |scheme,host,port,acc| > + provider = Factory :mock_provider, :name => "mock_provider" > + mock_account = Factory :mock_provider_account, :label => acc, :provider => provider > + params = {:host => host, :port => port, :provider_account => mock_account} > + if "https" == scheme > + params[:certificate] = "cert" > + end > + @config_server = Factory :mock_config_server, params > + # Don't particularly like this next bit, but the problem is the "create" > + # default_strategy used when instantiating the Factory leaves the > + # @config_server in a "SUCCESS" status. This bit effectively "resets" the > + # status to "UNTESTED" so the :test_connection method(stub) has to get called > + @config_server.stub!(:status).and_return(ConfigServer::ConnectionStatus.new()) > + case host > + when "bad_credentials" > + @config_server.stub!(:test_connection).and_raise(RestClient::Unauthorized) > + when "bad_host" > + @config_server.stub!(:test_connection).and_raise(Errno::ETIMEDOUT) > + end > + # ensure that the :mock_config_server (with stubbed methods) is returned > + ConfigServer.stub!(:find).and_return(@config_server) > +end > diff --git a/src/features/support/env.rb b/src/features/support/env.rb > index 33f9ed2..7357eda 100644 > --- a/src/features/support/env.rb > +++ b/src/features/support/env.rb > @@ -19,6 +19,9 @@ require File.expand_path(File.dirname(__FILE__) + '../../../spec/vcr_setup.rb') > require 'webrat' > require 'webrat/core/matchers' > > +# Pull in the ability to stub out methods > +require 'spec/stubs/cucumber' > + > Webrat.configure do |config| > config.mode = :rails > config.open_error_files = false # Set to true if you want error pages to pop up in the browser > diff --git a/src/features/support/paths.rb b/src/features/support/paths.rb > index 12a72d5..3a86c52 100644 > --- a/src/features/support/paths.rb > +++ b/src/features/support/paths.rb > @@ -98,6 +98,13 @@ module NavigationHelpers > when /^(.*)'s provider account page$/ > provider_account_path(ProviderAccount.find_by_label($1)) > > + when /the new config server page/ > + url_for new_config_server_path > + > + when /^the edit config server page/ > + @config_server.stub!(:connection_valid?).and_return(true) > + edit_config_server_path(@config_server) > + > when /the operational status of deployment page/ > deployment_path(@deployment, :details_tab => 'operation') > > @@ -123,7 +130,6 @@ module NavigationHelpers > when /^the (.*)'s edit user page$/ > edit_user_path(User.find_by_login($1)) > > - > # Add more mappings here. > # Here is an example that pulls values out of the Regexp: > # > diff --git a/src/spec/controllers/config_servers_controller_spec.rb b/src/spec/controllers/config_servers_controller_spec.rb > new file mode 100644 > index 0000000..9b2f1a0 > --- /dev/null > +++ b/src/spec/controllers/config_servers_controller_spec.rb > @@ -0,0 +1,104 @@ > +require 'spec_helper' > + > +describe ConfigServersController do > + fixtures :all > + > + before(:each) do > + @admin_permission = Factory :admin_permission > + @admin = @admin_permission.user > + activate_authlogic > + @session = UserSession.create(@admin) > + end > + > + context "editing config servers" do > + before(:each) do > + @config_server = Factory :mock_config_server > + @provider_account = @config_server.provider_account > + end > + > + it "should provide UI to edit an existing Config Server" do > + get :edit, :id => @config_server.id > + response.should be_success > + response.should render_template("edit") > + end > + > + it "should allow users with account modify permissions to update a Config Server" do > + post :update, :id => @config_server.id, :config_server => {:host => "host", :port => "port"} > + response.should be_success > + end > + end > + > + context "creating config servers" do > + before(:each) do > + @provider_account = Factory :mock_provider_account > + end > + > + it "should provide UI to create a new Config Server" do > + get :new, :provider_account_id => @provider_account.id > + response.should be_success > + response.should render_template("new") > + end > + > + it "should allow users with account modify permissions to create a Config Server" do > + config_server = Factory :mock_config_server, :host => "host", :port => "port" > + ConfigServer.stub!(:new).and_return(config_server) > + post :create, :provider_account_id => @provider_account.id, > + :config_server => { > + :host => "host", > + :port => "port" > + } > + response.should redirect_to(provider_account_path((a)provider_account.id)) > + response.flash[:error].should be_nil > + end > + > + it "should fail creating a config server when the username or password is invalid" do > + config_server = Factory :invalid_credentials_config_server, :host => "host", :port => "port", :username => "invalid", :password => "invalid" > + ConfigServer.stub!(:new).and_return(config_server) > + post :create, :provider_account_id => @provider_account.id, > + :config_server => { > + :host => "host", > + :port => "port", > + :username => "invalid", > + :password => "invalid" > + } > + response.should be_success > + response.should render_template("new") > + response.flash[:error].should == "The config server information is invalid." > + end > + > + it "should fail creating a config server when the host and port are invalid" do > + config_server = Factory :invalid_host_or_port_config_server, :host => "invalid", :port => "invalid" > + ConfigServer.stub!(:new).and_return(config_server) > + post :create, :provider_account_id => @provider_account.id, > + :config_server => { > + :host => "invalid", > + :port => "invalid" > + } > + response.should be_success > + response.should render_template("new") > + response.flash[:error].should == "The config server information is invalid." > + end > + > + it "should require that port is provided" do > + post :create, :provider_account_id => @provider_account.id, > + :config_server => { > + :host => "host", > + :port => "" > + } > + response.should be_success > + response.should render_template("new") > + response.flash[:error].should == "The config server information is invalid." > + end > + > + it "should require that host is provided" do > + post :create, :provider_account_id => @provider_account.id, > + :config_server => { > + :host => "", > + :port => "port" > + } > + response.should be_success > + response.should render_template("new") > + response.flash[:error].should == "The config server information is invalid." > + end > + end > +end > diff --git a/src/spec/factories/config_server.rb b/src/spec/factories/config_server.rb > new file mode 100644 > index 0000000..91e07f0 > --- /dev/null > +++ b/src/spec/factories/config_server.rb > @@ -0,0 +1,36 @@ > +Factory.define :config_server do |f| > + f.sequence(:host) {|n| "config_server#{n}" } > +end > + > +Factory.define :base_config_server, :parent => :config_server do |f| > + f.host "localhost" > + f.port "80" > + f.username "username" > + f.password "password" > +end > + > +Factory.define :mock_config_server, :parent => :base_config_server do |f| > + f.association :provider_account, :factory => :mock_provider_account > + f.after_build do |cs| > + cs.stub!(:test_connection).and_return(nil) if cs.respond_to? :stub! > + end > +end > + > +Factory.define :invalid_credentials_config_server, :parent => :base_config_server, :default_strategy => :build do |f| > + f.port "443" > + f.username "bad_username" > + f.password "bad_password" > + f.certificate "cert" > + f.after_build do |cs| > + cs.stub!(:test_connection).and_raise(RestClient::Unauthorized) if cs.respond_to? :stub! > + end > +end > + > +Factory.define :invalid_host_or_port_config_server, :parent => :base_config_server, :default_strategy => :build do |f| > + f.host "bad_host" > + f.port "443" > + f.certificate "cert" > + f.after_build do |cs| > + cs.stub!(:test_connection).and_raise(Errno::ETIMEDOUT) if cs.respond_to? :stub! > + end > +end > diff --git a/src/spec/models/config_server_spec.rb b/src/spec/models/config_server_spec.rb > new file mode 100644 > index 0000000..350f036 > --- /dev/null > +++ b/src/spec/models/config_server_spec.rb > @@ -0,0 +1,42 @@ > +require 'spec_helper' > + > +describe ConfigServer do > + describe "standard behavior" do > + before(:each) do > + @config_server = Factory.build :mock_config_server > + end > + > + it "should require a host" do > + @config_server.should be_valid > + @config_server.host = nil > + @config_server.should_not be_valid > + end > + > + it "should require a port" do > + @config_server.should be_valid > + @config_server.port = nil > + @config_server.should_not be_valid > + end > + > + it "should suggest https when a cert is present" do > + @config_server.certificate = "abc" > + @config_server.base_url.should =~ /https:\/\/.*/ > + end > + > + it "should suggest http when a cert is not present" do > + @config_server.certificate = nil > + @config_server.base_url.should =~ /http:\/\/.*/ > + end > + end > + > + describe "error behavior: invalid credentials" do > + before(:each) do > + @config_server = Factory.build :invalid_credentials_config_server > + end > + > + it "should report an error when unauthorized" do > + @config_server.should_not be_valid > + @config_server.errors.full_messages.join(" ").should include_text("Could not validate config server connection") > + end > + end > +end > diff --git a/src/spec/models/provider_account_spec.rb b/src/spec/models/provider_account_spec.rb > index 370a132..d3c53b8 100644 > --- a/src/spec/models/provider_account_spec.rb > +++ b/src/spec/models/provider_account_spec.rb > @@ -18,6 +18,13 @@ describe ProviderAccount do > @provider_account.should be_frozen > end > > + it "should be destroyable if it has a config server" do > + @provider_account.config_server = ConfigServer.new > + @provider_account.destroyable?.should be_true > + @provider_account.destroy.equal?((a)provider_account).should be_true > + @provider_account.should be_frozen > + end > + > it "should check the validitiy of the cloud account login credentials" do > mock_provider = Factory :mock_provider > ACK and pushed. Great work! Thomas

On 07/18/2011 11:36 PM, Mike Orazi wrote: > On 07/18/2011 10:02 AM, Tomas Sedovic wrote: >> On 07/14/2011 05:14 PM, gblomqui(a)redhat.com wrote: >>> From: Greg Blomquist&lt;gblomqui(a)redhat.com&gt; >>> >>> This commit adds the ability to manage config server data as an object >>> associated to provider accounts. Specifically, provider accounts can have >>> 0-or-1 config servers. >>> >>> Now, a user can add, edit, delete, and test a config server from the provider account >>> view. >>> >>> Signed-off-by: Greg Blomquist&lt;gblomqui(a)redhat.com&gt; >>> --- >>> src/app/controllers/config_servers_controller.rb | 74 ++++++++++ >>> src/app/models/config_server.rb | 146 ++++++++++++++++++++ >>> src/app/models/provider_account.rb | 2 + >>> .../views/config_servers/_config_server_form.haml | 22 +++ >>> src/app/views/config_servers/_section_header.haml | 3 + >>> src/app/views/config_servers/edit.haml | 21 +++ >>> src/app/views/config_servers/new.haml | 16 ++ >>> src/app/views/provider_accounts/_properties.haml | 22 +++ >>> src/config/locales/en.yml | 14 ++ >>> src/config/routes.rb | 1 + >>> .../migrate/20110606141425_create_config_server.rb | 18 +++ >>> src/features/config_server.feature | 109 +++++++++++++++ >>> .../step_definitions/config_server_steps.rb | 37 +++++ >>> src/features/support/env.rb | 3 + >>> src/features/support/paths.rb | 8 +- >>> .../controllers/config_servers_controller_spec.rb | 104 ++++++++++++++ >>> src/spec/factories/config_server.rb | 36 +++++ >>> src/spec/models/config_server_spec.rb | 42 ++++++ >>> src/spec/models/provider_account_spec.rb | 7 + >>> 19 files changed, 684 insertions(+), 1 deletions(-) >>> create mode 100644 src/app/controllers/config_servers_controller.rb >>> create mode 100644 src/app/models/config_server.rb >>> create mode 100644 src/app/views/config_servers/_config_server_form.haml >>> create mode 100644 src/app/views/config_servers/_section_header.haml >>> create mode 100644 src/app/views/config_servers/edit.haml >>> create mode 100644 src/app/views/config_servers/new.haml >>> create mode 100644 src/db/migrate/20110606141425_create_config_server.rb >>> create mode 100644 src/features/config_server.feature >>> create mode 100644 src/features/step_definitions/config_server_steps.rb >>> create mode 100644 src/spec/controllers/config_servers_controller_spec.rb >>> create mode 100644 src/spec/factories/config_server.rb >>> create mode 100644 src/spec/models/config_server_spec.rb >>> >>> diff --git a/src/app/controllers/config_servers_controller.rb b/src/app/controllers/config_servers_controller.rb >>> new file mode 100644 >>> index 0000000..dd53be1 >>> --- /dev/null >>> +++ b/src/app/controllers/config_servers_controller.rb >>> @@ -0,0 +1,74 @@ >>> +require 'rest_client' >>> + >>> +class ConfigServersController< ApplicationController >>> + before_filter :require_user >>> + layout 'application' >>> + >>> + def top_section >>> + :administer >>> + end >>> + >>> + def edit >>> + @config_server = ConfigServer.find(params[:id]) >>> + @provider_account = @config_server.provider_account >>> + require_privilege(Privilege::MODIFY, @provider_account) >>> + end >>> + >>> + def new >>> + @provider_account = ProviderAccount.find(params[:provider_account_id]) >>> + require_privilege(Privilege::MODIFY, @provider_account) >>> + @config_server = ConfigServer.new() >>> + end >>> + >>> + def test >>> + config_server = ConfigServer.find(params[:id]) >>> + if not config_server.connection_valid? >>> + flash[:error] = config_server.connection_error_msg >>> + else >>> + flash[:notice] = "Test successful" >>> + end >>> + redirect_to provider_account_path(config_server.provider_account.id) >>> + end >>> + >>> + def create >>> + @provider_account = ProviderAccount.find(params[:provider_account_id]) >>> + # for now the privileges required to create, modify, or delete a config >>> + # server are tied to modifying the particular associated provider account >>> + require_privilege(Privilege::MODIFY, @provider_account) >>> + >>> + @config_server = ConfigServer.new(params[:config_server]) >>> + @config_server.provider_account = @provider_account >>> + if @config_server.invalid? >>> + flash[:error] = "The config server information is invalid." >>> + render :action => 'new' and return >>> + end >>> + @config_server.save! >>> + flash[:notice] = "Config server added." >>> + redirect_to provider_account_path(@provider_account) >>> + end >>> + >>> + def update >>> + @config_server = ConfigServer.find(params[:id]) >>> + @provider_account = @config_server.provider_account >>> + require_privilege(Privilege::MODIFY, @provider_account) >>> + >>> + if @config_server.update_attributes(params[:config_server]) >>> + flash[:notice] = "Config server updated." >>> + redirect_to provider_account_path(@provider_account) >>> + else >>> + flash[:error] = "Config server was not updated" >>> + render :action => :edit >>> + end >>> + end >>> + >>> + def destroy >>> + @config_server = ConfigServer.find(params[:id]) >>> + require_privilege(Privilege::MODIFY, @config_server.provider_account) >>> + if ConfigServer.destroy(params[:id]) >>> + flash[:notice] = "Config server was deleted." >>> + else >>> + flash[:error] = "Config server was not deleted" >>> + end >>> + redirect_to provider_account_path((a)config_server.provider_account) >>> + end >>> +end >>> diff --git a/src/app/models/config_server.rb b/src/app/models/config_server.rb >>> new file mode 100644 >>> index 0000000..9ccba5f >>> --- /dev/null >>> +++ b/src/app/models/config_server.rb >>> @@ -0,0 +1,146 @@ >>> +# == Schema Information >>> +# Schema version: 20110517095823 >>> +# >>> +# Table name: config_servers >>> +# >>> +# id :integer not null, primary key >>> +# host :string(255) not null >>> +# port :string(255) not null >>> +# username :string(255) null >>> +# password :string(255) null >>> +# certificate :string(255) null >>> +# provider_id :integer not null, fk_provider >>> +# >>> + >>> +## Copyright (C) 2011 Red Hat, Inc. >>> +## Written by Greg Blomquist&lt;gblomqui(a)redhat.com&gt; >>> +## >>> +## This program is free software; you can redistribute it and/or modify >>> +## it under the terms of the GNU General Public License as published by >>> +## the Free Software Foundation; version 2 of the License. >>> +## >>> +## This program is distributed in the hope that it will be useful, >>> +## but WITHOUT ANY WARRANTY; without even the implied warranty of >>> +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> +## GNU General Public License for more details. >>> +## >>> +## You should have received a copy of the GNU General Public License >>> +## along with this program; if not, write to the Free Software >>> +## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, >>> +## MA 02110-1301, USA. A copy of the GNU General Public License is >>> +## also available at http://www.gnu.org/copyleft/gpl.html. >>> +# >>> + >>> +class ConfigServer< ActiveRecord::Base >>> + >>> + class ConnectionStatus >>> + attr_reader :state, :message >>> + def initialize >>> + @state = :UNTESTED >>> + end >>> + >>> + def untested? >>> + return :UNTESTED == @state >>> + end >>> + >>> + def fail!(message) >>> + @state = :FAILURE >>> + @message = message >>> + end >>> + def fail? >>> + return :FAILURE == @state >>> + end >>> + >>> + def success!(message=nil) >>> + @state = :SUCCESS >>> + @message = message >>> + end >>> + def success? >>> + return :SUCCESS == @state >>> + end >>> + end >>> + @@status_fields = [:host, :port, :username, :password, :certificate] >>> + >>> + belongs_to :provider_account >>> + >>> + validates_presence_of :host >>> + validates_presence_of :port >>> + validate :validate_connection >>> + >>> + # Utility method to piece together the connection information for this config >>> + # server. >>> + def base_url >>> + scheme = (certificate.blank?) ? "http" : "https" >>> + domain = host >>> + if not port.blank? >>> + domain += ":#{port}" >>> + end >>> + "#{scheme}://#{domain}" >>> + end >>> + >>> + # Reports the error message (if any) produced by testing the connection to >>> + # this config server. >>> + def connection_error_msg >>> + if not connection_valid? >>> + return status.message >>> + end >>> + end >>> + >>> + # Determines if the connection represented by #host, #port, #username, >>> + # #password, and #certificate is a valid connection. If the connection is >>> + # invalid, calling #connection_error_msg will return the error that was >>> + # generated when testing the connection. >>> + def connection_valid? >>> + if status.untested? or (changed? and (changes.keys& @(a)status_fields).empty?) >>> + begin >>> + test_connection >>> + status.success! >>> + rescue => e >>> + error_str = map_connection_exception_to_error(e) >>> + status.fail!(error_str) >>> + end >>> + end >>> + status.success? >>> + end >>> + >>> + private >>> + def status >>> + # smells like a factory in here >>> + @status ||= ConnectionStatus.new >>> + end >>> + >>> + def validate_connection >>> + # for validation hook >>> + if not connection_valid? >>> + errors.add_to_base(status.message) >>> + end >>> + end >>> + >>> + # Test the connection to this config server. Return nil on success, or throw >>> + # an exception on errors. See >>> + # http://rubydoc.info/gems/rest-client/1.6.3/RestClient#STATUSES-constant >>> + # for more information on the types of exceptions. >>> + def test_connection >>> + args = {:method => :get} >>> + if not username.blank? >>> + args[:user] = username >>> + args[:password] = password >>> + end >>> + if not certificate.blank? >>> + args[:ssl_client_cert] = OpenSSL::X509::Certificate.new(certificate) >>> + end >>> + args[:url] = "#{base_url}/version" >>> + args[:raw_response] = true >>> + RestClient::Request.execute(args) >>> + end >>> + >>> + def map_connection_exception_to_error(ex) >>> + if ex.class == OpenSSL::X509::CertificateError >>> + error_string = I18n.translate("config_servers.errors.connection.certificate", :url => base_url, :msg => ex.message) >>> + elsif ex.kind_of?(RestClient::ExceptionWithResponse) or ex.kind_of?(RestClient::Exception) or ex.class == Errno::ETIMEDOUT >>> + error_string = I18n.translate("config_servers.errors.connection.generic_with_message", :url => base_url, :msg => ex.message) >>> + elsif not ex.nil? >>> + error_string = I18n.translate("config_servers.errors.connection.generic", :url => base_url) >>> + end >>> + end >>> +end >>> diff --git a/src/app/models/provider_account.rb b/src/app/models/provider_account.rb >>> index 812d5a1..3cd3691 100644 >>> --- a/src/app/models/provider_account.rb >>> +++ b/src/app/models/provider_account.rb >>> @@ -51,6 +51,8 @@ class ProviderAccount< ActiveRecord::Base >>> # validation of credentials is done in provider_account validation, :validate => false prevents nested_attributes from validation >>> has_many :credentials, :dependent => :destroy, :validate => false >>> accepts_nested_attributes_for :credentials >>> + # eventually, this might be "has_many", but first pass is one-to-one >>> + has_one :config_server, :dependent => :destroy >>> >>> # Helpers >>> attr_accessor :x509_cert_priv_file, :x509_cert_pub_file >>> diff --git a/src/app/views/config_servers/_config_server_form.haml b/src/app/views/config_servers/_config_server_form.haml >>> new file mode 100644 >>> index 0000000..5e60896 >>> --- /dev/null >>> +++ b/src/app/views/config_servers/_config_server_form.haml >>> @@ -0,0 +1,22 @@ >>> +%fieldset.clearfix >>> + - if not @config_server.id.nil? >>> + = hidden_field_tag "config_server[id]", @config_server.id >>> + = hidden_field_tag "provider_account_id", @provider_account.id >>> + = label_tag "config_server[host]", "Host: " >>> + = text_field_tag "config_server[host]", @config_server.host >>> +%fieldset.clearfix >>> + = label_tag "config_server[port]", "Port: " >>> + = text_field_tag "config_server[port]", @config_server.port >>> +%fieldset.clearfix >>> + = label_tag "config_server[username]", "Username: " >>> + = text_field_tag "config_server[username]", @config_server.username >>> +%fieldset.clearfix >>> + = label_tag "config_server[password]", "Password: " >>> + = password_field_tag "config_server[password]", @config_server.password >>> +%fieldset.clearfix >>> + = label_tag "config_server[certificate]", "Certificate: " >>> + = text_area_tag "config_server[certificate]", @config_server.certificate, :rows => 20, :cols => 64 >>> + = t('config_servers.certificate_help') >>> +%fieldset.clearfix >>> + .grid_13.alpha.omega >>> + = submit_tag t(:save), :class => "ra nomargin dialogbutton" >>> diff --git a/src/app/views/config_servers/_section_header.haml b/src/app/views/config_servers/_section_header.haml >>> new file mode 100644 >>> index 0000000..d8e75fe >>> --- /dev/null >>> +++ b/src/app/views/config_servers/_section_header.haml >>> @@ -0,0 +1,3 @@ >>> +%header.page-header >>> + %h1{:class => controller.controller_name} Config Server >>> + .corner&nbsp; >>> diff --git a/src/app/views/config_servers/edit.haml b/src/app/views/config_servers/edit.haml >>> new file mode 100644 >>> index 0000000..42e40f8 >>> --- /dev/null >>> +++ b/src/app/views/config_servers/edit.haml >>> @@ -0,0 +1,21 @@ >>> += render :partial => 'layouts/admin_header' >>> +%header.page-header >>> + %h1{:class => controller.controller_name}= &quot;#{(a)provider_account.name} Config Server" >>> + #obj_actions.button-container >>> + %div.button-group >>> + = link_to 'Cancel Editing', provider_account_path(@provider_account), :class => 'button pill danger', :id => 'cancel_config_server_edit' >>> + .corner&nbsp; >>> + >>> +%section.content-section.config_server >>> + %header >>> + %h2 Edit Config Server >>> + >>> + .content >>> + = error_messages_for :config_server >>> + %h2 >>> + = t('config_servers.edit.edit_config_server') >>> + - form_tag(config_server_path, { :method => :put }) do >>> + = render :partial => 'config_server_form', :locals => {:from => "edit"} >>> + -#%fieldset.clearfix >>> + .grid_13.alpha.omega >>> + = submit_tag t(:save), :class => "ra nomargin dialogbutton" >>> diff --git a/src/app/views/config_servers/new.haml b/src/app/views/config_servers/new.haml >>> new file mode 100644 >>> index 0000000..c21aee4 >>> --- /dev/null >>> +++ b/src/app/views/config_servers/new.haml >>> @@ -0,0 +1,16 @@ >>> += render :partial => 'layouts/admin_header' >>> += render :partial => 'section_header' >>> + >>> +%section.content-section.config_server >>> + %header >>> + %h2 New Config Server >>> + >>> + .content >>> + = error_messages_for :config_server >>> + %h2 >>> + = t('config_servers.new.new_config_server') >>> + - form_for(@config_server, :url => config_servers_path) do |f| >>> + = render :partial => 'config_server_form', :locals => {:from => "new"} >>> + -#%fieldset >>> + .grid_13.alpha.omega >>> + = f.submit t(:add), :class => "ra nomargin dialogbutton" >>> diff --git a/src/app/views/provider_accounts/_properties.haml b/src/app/views/provider_accounts/_properties.haml >>> index 559bdf1..6990a17 100644 >>> --- a/src/app/views/provider_accounts/_properties.haml >>> +++ b/src/app/views/provider_accounts/_properties.haml >>> @@ -17,3 +17,25 @@ >>> = label_tag "Account number:" >>> %td >>> = @account_id >>> + %tr >>> + %td >>> + %label Config Server: >>> + %td >>> + - missing_config_server = @account.config_server.nil? >>> + %span#config_server >>> + = missing_config_server ? "None" : @account.config_server.base_url >>> + %span#config_server_control >>> + - if missing_config_server >>> + [ >>> + = link_to 'Add', new_config_server_url + &quot;?provider_account_id=#{(a)account.id}&quot; >>> + ] >>> + - else >>> + [ >>> + = link_to 'Edit', edit_config_server_path((a)account.config_server) >>> + ] >>> + [ >>> + = link_to 'Test', test_config_servers_path + &quot;?id=#{(a)account.config_server.id}&quot; >>> + ] >>> + [ >>> + = link_to 'Delete', config_server_path((a)account.config_server), :method => 'delete', :confirm => "Are you sure you want to delete this config server?" >>> + ] >>> diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml >>> index 1564983..9af4ab8 100644 >>> --- a/src/config/locales/en.yml >>> +++ b/src/config/locales/en.yml >>> @@ -237,6 +237,8 @@ en: >>> account_not_added: >>> one: "Account %{list} could not be added" >>> other: "Accounts %{list} could not be added" >>> + config_server_saved: "Config Server data was successfully saved" >>> + config_server_not_saved: "An error occurred while saving the Config Server data." >>> new: >>> new_provider_account: New Account >>> required_field: Required field. >>> @@ -254,4 +256,16 @@ en: >>> account_number: AWS Account ID >>> account_private_cert: EC2 x509 private key >>> account_public_cert: EC2 x509 public key >>> + config_servers: >>> + certificate_help: Provide a certificate to enable https support >>> + new: >>> + new_config_server: New Config Server >>> + edit: >>> + edit_config_server: Edit Config Server >>> + errors: >>> + connection: >>> + generic: "Could not validate config server connection (url: %{url}). Check the connection parameters and try again." >>> + generic_with_message: "Could not validate config server connection (url: %{url}). %{msg}" >>> + certificate: "Could not validate config server connection (url: %{url}). Certificate error: %{msg}" >>> + unhandled_response: "Could not validate config server connection (url: %{url}). HTTP Status Code: %{code}" >>> uptime: Uptime >>> diff --git a/src/config/routes.rb b/src/config/routes.rb >>> index 7f83338..18ae28d 100644 >>> --- a/src/config/routes.rb >>> +++ b/src/config/routes.rb >>> @@ -46,6 +46,7 @@ ActionController::Routing::Routes.draw do |map| >>> map.resources :provider_types, :only => :index >>> map.resources :users, :collection => { :multi_destroy => :delete } >>> map.resources :provider_accounts, :collection => { :multi_destroy => :delete, :set_selected_provider => :get} >>> + map.resources :config_servers, :collection => { :test => :get } >>> map.resources :roles, :collection => { :multi_destroy => :delete } >>> map.resources :settings, :collection => { :self_service => :get, :general_settings => :get } >>> map.resources :pool_families, :collection => { :multi_destroy => :delete, :add_provider_account => :post, :multi_destroy_provider_accounts => :delete } >>> diff --git a/src/db/migrate/20110606141425_create_config_server.rb b/src/db/migrate/20110606141425_create_config_server.rb >>> new file mode 100644 >>> index 0000000..e10ce78 >>> --- /dev/null >>> +++ b/src/db/migrate/20110606141425_create_config_server.rb >>> @@ -0,0 +1,18 @@ >>> +class CreateConfigServer< ActiveRecord::Migration >>> + def self.up >>> + create_table :config_servers do |t| >>> + t.string :host, :null => false >>> + t.string :port, :null => false >>> + t.string :username, :null => true >>> + t.string :password, :null => true >>> + t.string :certificate, :null => true, :limit => 2048 >>> + t.integer :provider_account_id, :null => false >>> + >>> + t.timestamps >>> + end >>> + end >>> + >>> + def self.down >>> + drop_table :config_servers >>> + end >>> +end >>> diff --git a/src/features/config_server.feature b/src/features/config_server.feature >>> new file mode 100644 >>> index 0000000..9beecf8 >>> --- /dev/null >>> +++ b/src/features/config_server.feature >>> @@ -0,0 +1,109 @@ >>> +Feature: Config Servers >>> + In order to administer configuration management on systems >>> + As a user >>> + I want to manage config servers as part of provider accounts >>> + >>> + Background: >>> + Given I am an authorised user >>> + And I am logged in >>> + >>> + # This scenario relies on a stubbed version of ConfigServer >>> + # that never fails on the connection test >>> + Scenario: I am able to add a config server to a provider account >>> + Given I am on the homepage >>> + And there is mock provider account "mock_account" >>> + And I want to add a new config server >>> + When I go to mock_account's provider account page >>> + Then I should see "None" within "#config_server" >>> + And I should see "[ Add ]" within "#config_server_control" >>> + When I follow "Add" >>> + Then I should be on the new config server page >>> + When I fill in "config_server[host]" with "valid_host" >>> + When I fill in "config_server[port]" with "valid_port" >>> + And I press "Save" >>> + Then I should be on mock_account's provider account page >>> + And I should see "Config server added" >>> + And I should see "[ Edit ]" within "#config_server_control" >>> + >>> + # This is essentially the same scenario as the first, but creates >>> + # a different stubbed ConfigServer, so it fails >>> + Scenario: I cannot add a config server with invalid host or port information >>> + Given I am on the homepage >>> + And there is mock provider account "mock_account" >>> + And I am not sure about the config server host >>> + When I go to mock_account's provider account page >>> + Then I should see "None" within "#config_server" >>> + And I should see "[ Add ]" within "#config_server_control" >>> + When I follow "Add" >>> + Then I should be on the new config server page >>> + When I fill in "config_server[host]" with "invalid" >>> + When I fill in "config_server[port]" with "invalid" >>> + And I press "Save" >>> + Then I should see "The config server information is invalid" >>> + And I should see "Could not validate config server connection" >>> + >>> + # This is essentially the same scenario as the first, but creates >>> + # a different stubbed ConfigServer, so it fails >>> + Scenario: I cannot add a config server with invalid credentials >>> + Given I am on the homepage >>> + And there is mock provider account "mock_account" >>> + And I am not sure about the config server credentials >>> + When I go to mock_account's provider account page >>> + Then I should see "None" within "#config_server" >>> + And I should see "[ Add ]" within "#config_server_control" >>> + When I follow "Add" >>> + Then I should be on the new config server page >>> + When I fill in "config_server[host]" with "valid" >>> + When I fill in "config_server[port]" with "valid" >>> + When I fill in "config_server[username]" with "invalid" >>> + When I fill in "config_server[password]" with "invalid" >>> + And I press "Save" >>> + Then I should see "The config server information is invalid" >>> + And I should see "Could not validate config server connection" >>> + >>> + Scenario: I should be able to edit a config server >>> + Given I am on the homepage >>> + And there is a mock config server "https://mock:443" for account "mock_account" >>> + When I go to mock_account's provider account page >>> + Then I should see "[ Edit ]" within "#config_server_control" >>> + When I follow "Edit" within "#config_server_control" >>> + Then I should be on the edit config server page for account "mock_account" >>> + And I press "Save" >>> + Then I should be on mock_account's provider account page >>> + And I should see "Config server updated" >>> + >>> + Scenario: I should be able to delete an existing config server >>> + Given I am on the homepage >>> + And there is a mock config server "https://mock:443" for account "mock_account" >>> + When I go to mock_account's provider account page >>> + Then I should see "[ Delete ]" within "#config_server_control" >>> + When I follow "Delete" within "#config_server_control" >>> + Then I should see "Config server was deleted" >>> + And I should be on mock_account's provider account page >>> + >>> + Scenario: I should be able to test a correctly configured and available config server >>> + Given I am on the homepage >>> + And there is a mock config server "https://mock:443" for account "mock_account" >>> + When I go to mock_account's provider account page >>> + Then I should see "[ Test ]" within "#config_server_control" >>> + When I follow "Test" within "#config_server_control" >>> + Then I should see "Test successful" >>> + And I should be on mock_account's provider account page >>> + >>> + Scenario: I should see an error when I test a config server with invalid credentials >>> + Given I am on the homepage >>> + And there is a mock config server "https://bad_credentials:443" for account "mock_account" >>> + When I go to mock_account's provider account page >>> + Then I should see "[ Test ]" within "#config_server_control" >>> + When I follow "Test" within "#config_server_control" >>> + Then I should see "Could not validate config server connection" >>> + And I should see "Unauthorized" >>> + >>> + Scenario: I should see an error when I test a config server with an invalid host >>> + Given I am on the homepage >>> + And there is a mock config server "https://bad_host:443" for account "mock_account" >>> + When I go to mock_account's provider account page >>> + Then I should see "[ Test ]" within "#config_server_control" >>> + When I follow "Test" within "#config_server_control" >>> + Then I should see "Could not validate config server connection" >>> + And I should see "Connection timed out" >>> diff --git a/src/features/step_definitions/config_server_steps.rb b/src/features/step_definitions/config_server_steps.rb >>> new file mode 100644 >>> index 0000000..438ad00 >>> --- /dev/null >>> +++ b/src/features/step_definitions/config_server_steps.rb >>> @@ -0,0 +1,37 @@ >>> +Given /^I want to add a new config server$/ do >>> + c = Factory.build(:mock_config_server) >>> + ConfigServer.stub!(:new).and_return(c) >>> +end >>> + >>> +Given /^I am not sure about the config server (host|port)$/ do |host_or_port| >>> + c = Factory.build(:invalid_host_or_port_config_server) >>> + ConfigServer.stub!(:new).and_return(c) >>> +end >>> + >>> +Given /^I am not sure about the config server credentials$/ do >>> + c = Factory.build(:invalid_credentials_config_server) >>> + ConfigServer.stub!(:new).and_return(c) >>> +end >>> + >>> +Given /^there is a mock config server "(http|https):\/\/(.*):(.*)" for account "(.*)"$/ do |scheme,host,port,acc| >>> + provider = Factory :mock_provider, :name => "mock_provider" >>> + mock_account = Factory :mock_provider_account, :label => acc, :provider => provider >>> + params = {:host => host, :port => port, :provider_account => mock_account} >>> + if "https" == scheme >>> + params[:certificate] = "cert" >>> + end >>> + @config_server = Factory :mock_config_server, params >>> + # Don't particularly like this next bit, but the problem is the "create" >>> + # default_strategy used when instantiating the Factory leaves the >>> + # @config_server in a "SUCCESS" status. This bit effectively "resets" the >>> + # status to "UNTESTED" so the :test_connection method(stub) has to get called >>> + @config_server.stub!(:status).and_return(ConfigServer::ConnectionStatus.new()) >>> + case host >>> + when "bad_credentials" >>> + @config_server.stub!(:test_connection).and_raise(RestClient::Unauthorized) >>> + when "bad_host" >>> + @config_server.stub!(:test_connection).and_raise(Errno::ETIMEDOUT) >>> + end >>> + # ensure that the :mock_config_server (with stubbed methods) is returned >>> + ConfigServer.stub!(:find).and_return(@config_server) >>> +end >>> diff --git a/src/features/support/env.rb b/src/features/support/env.rb >>> index 33f9ed2..7357eda 100644 >>> --- a/src/features/support/env.rb >>> +++ b/src/features/support/env.rb >>> @@ -19,6 +19,9 @@ require File.expand_path(File.dirname(__FILE__) + '../../../spec/vcr_setup.rb') >>> require 'webrat' >>> require 'webrat/core/matchers' >>> >>> +# Pull in the ability to stub out methods >>> +require 'spec/stubs/cucumber' >>> + >>> Webrat.configure do |config| >>> config.mode = :rails >>> config.open_error_files = false # Set to true if you want error pages to pop up in the browser >>> diff --git a/src/features/support/paths.rb b/src/features/support/paths.rb >>> index 12a72d5..3a86c52 100644 >>> --- a/src/features/support/paths.rb >>> +++ b/src/features/support/paths.rb >>> @@ -98,6 +98,13 @@ module NavigationHelpers >>> when /^(.*)'s provider account page$/ >>> provider_account_path(ProviderAccount.find_by_label($1)) >>> >>> + when /the new config server page/ >>> + url_for new_config_server_path >>> + >>> + when /^the edit config server page/ >>> + @config_server.stub!(:connection_valid?).and_return(true) >>> + edit_config_server_path(@config_server) >>> + >>> when /the operational status of deployment page/ >>> deployment_path(@deployment, :details_tab => 'operation') >>> >>> @@ -123,7 +130,6 @@ module NavigationHelpers >>> when /^the (.*)'s edit user page$/ >>> edit_user_path(User.find_by_login($1)) >>> >>> - >>> # Add more mappings here. >>> # Here is an example that pulls values out of the Regexp: >>> # >>> diff --git a/src/spec/controllers/config_servers_controller_spec.rb b/src/spec/controllers/config_servers_controller_spec.rb >>> new file mode 100644 >>> index 0000000..9b2f1a0 >>> --- /dev/null >>> +++ b/src/spec/controllers/config_servers_controller_spec.rb >>> @@ -0,0 +1,104 @@ >>> +require 'spec_helper' >>> + >>> +describe ConfigServersController do >>> + fixtures :all >>> + >>> + before(:each) do >>> + @admin_permission = Factory :admin_permission >>> + @admin = @admin_permission.user >>> + activate_authlogic >>> + @session = UserSession.create(@admin) >>> + end >>> + >>> + context "editing config servers" do >>> + before(:each) do >>> + @config_server = Factory :mock_config_server >>> + @provider_account = @config_server.provider_account >>> + end >>> + >>> + it "should provide UI to edit an existing Config Server" do >>> + get :edit, :id => @config_server.id >>> + response.should be_success >>> + response.should render_template("edit") >>> + end >>> + >>> + it "should allow users with account modify permissions to update a Config Server" do >>> + post :update, :id => @config_server.id, :config_server => {:host => "host", :port => "port"} >>> + response.should be_success >>> + end >>> + end >>> + >>> + context "creating config servers" do >>> + before(:each) do >>> + @provider_account = Factory :mock_provider_account >>> + end >>> + >>> + it "should provide UI to create a new Config Server" do >>> + get :new, :provider_account_id => @provider_account.id >>> + response.should be_success >>> + response.should render_template("new") >>> + end >>> + >>> + it "should allow users with account modify permissions to create a Config Server" do >>> + config_server = Factory :mock_config_server, :host => "host", :port => "port" >>> + ConfigServer.stub!(:new).and_return(config_server) >>> + post :create, :provider_account_id => @provider_account.id, >>> + :config_server => { >>> + :host => "host", >>> + :port => "port" >>> + } >>> + response.should redirect_to(provider_account_path((a)provider_account.id)) >>> + response.flash[:error].should be_nil >>> + end >>> + >>> + it "should fail creating a config server when the username or password is invalid" do >>> + config_server = Factory :invalid_credentials_config_server, :host => "host", :port => "port", :username => "invalid", :password => "invalid" >>> + ConfigServer.stub!(:new).and_return(config_server) >>> + post :create, :provider_account_id => @provider_account.id, >>> + :config_server => { >>> + :host => "host", >>> + :port => "port", >>> + :username => "invalid", >>> + :password => "invalid" >>> + } >>> + response.should be_success >>> + response.should render_template("new") >>> + response.flash[:error].should == "The config server information is invalid." >>> + end >>> + >>> + it "should fail creating a config server when the host and port are invalid" do >>> + config_server = Factory :invalid_host_or_port_config_server, :host => "invalid", :port => "invalid" >>> + ConfigServer.stub!(:new).and_return(config_server) >>> + post :create, :provider_account_id => @provider_account.id, >>> + :config_server => { >>> + :host => "invalid", >>> + :port => "invalid" >>> + } >>> + response.should be_success >>> + response.should render_template("new") >>> + response.flash[:error].should == "The config server information is invalid." >>> + end >>> + >>> + it "should require that port is provided" do >>> + post :create, :provider_account_id => @provider_account.id, >>> + :config_server => { >>> + :host => "host", >>> + :port => "" >>> + } >>> + response.should be_success >>> + response.should render_template("new") >>> + response.flash[:error].should == "The config server information is invalid." >>> + end >>> + >>> + it "should require that host is provided" do >>> + post :create, :provider_account_id => @provider_account.id, >>> + :config_server => { >>> + :host => "", >>> + :port => "port" >>> + } >>> + response.should be_success >>> + response.should render_template("new") >>> + response.flash[:error].should == "The config server information is invalid." >>> + end >>> + end >>> +end >>> diff --git a/src/spec/factories/config_server.rb b/src/spec/factories/config_server.rb >>> new file mode 100644 >>> index 0000000..91e07f0 >>> --- /dev/null >>> +++ b/src/spec/factories/config_server.rb >>> @@ -0,0 +1,36 @@ >>> +Factory.define :config_server do |f| >>> + f.sequence(:host) {|n| "config_server#{n}" } >>> +end >>> + >>> +Factory.define :base_config_server, :parent => :config_server do |f| >>> + f.host "localhost" >>> + f.port "80" >>> + f.username "username" >>> + f.password "password" >>> +end >>> + >>> +Factory.define :mock_config_server, :parent => :base_config_server do |f| >>> + f.association :provider_account, :factory => :mock_provider_account >>> + f.after_build do |cs| >>> + cs.stub!(:test_connection).and_return(nil) if cs.respond_to? :stub! >>> + end >>> +end >>> + >>> +Factory.define :invalid_credentials_config_server, :parent => :base_config_server, :default_strategy => :build do |f| >>> + f.port "443" >>> + f.username "bad_username" >>> + f.password "bad_password" >>> + f.certificate "cert" >>> + f.after_build do |cs| >>> + cs.stub!(:test_connection).and_raise(RestClient::Unauthorized) if cs.respond_to? :stub! >>> + end >>> +end >>> + >>> +Factory.define :invalid_host_or_port_config_server, :parent => :base_config_server, :default_strategy => :build do |f| >>> + f.host "bad_host" >>> + f.port "443" >>> + f.certificate "cert" >>> + f.after_build do |cs| >>> + cs.stub!(:test_connection).and_raise(Errno::ETIMEDOUT) if cs.respond_to? :stub! >>> + end >>> +end >>> diff --git a/src/spec/models/config_server_spec.rb b/src/spec/models/config_server_spec.rb >>> new file mode 100644 >>> index 0000000..350f036 >>> --- /dev/null >>> +++ b/src/spec/models/config_server_spec.rb >>> @@ -0,0 +1,42 @@ >>> +require 'spec_helper' >>> + >>> +describe ConfigServer do >>> + describe "standard behavior" do >>> + before(:each) do >>> + @config_server = Factory.build :mock_config_server >>> + end >>> + >>> + it "should require a host" do >>> + @config_server.should be_valid >>> + @config_server.host = nil >>> + @config_server.should_not be_valid >>> + end >>> + >>> + it "should require a port" do >>> + @config_server.should be_valid >>> + @config_server.port = nil >>> + @config_server.should_not be_valid >>> + end >>> + >>> + it "should suggest https when a cert is present" do >>> + @config_server.certificate = "abc" >>> + @config_server.base_url.should =~ /https:\/\/.*/ >>> + end >>> + >>> + it "should suggest http when a cert is not present" do >>> + @config_server.certificate = nil >>> + @config_server.base_url.should =~ /http:\/\/.*/ >>> + end >>> + end >>> + >>> + describe "error behavior: invalid credentials" do >>> + before(:each) do >>> + @config_server = Factory.build :invalid_credentials_config_server >>> + end >>> + >>> + it "should report an error when unauthorized" do >>> + @config_server.should_not be_valid >>> + @config_server.errors.full_messages.join(" ").should include_text("Could not validate config server connection") >>> + end >>> + end >>> +end >>> diff --git a/src/spec/models/provider_account_spec.rb b/src/spec/models/provider_account_spec.rb >>> index 370a132..d3c53b8 100644 >>> --- a/src/spec/models/provider_account_spec.rb >>> +++ b/src/spec/models/provider_account_spec.rb >>> @@ -18,6 +18,13 @@ describe ProviderAccount do >>> @provider_account.should be_frozen >>> end >>> >>> + it "should be destroyable if it has a config server" do >>> + @provider_account.config_server = ConfigServer.new >>> + @provider_account.destroyable?.should be_true >>> + @provider_account.destroy.equal?((a)provider_account).should be_true >>> + @provider_account.should be_frozen >>> + end >>> + >>> it "should check the validitiy of the cloud account login credentials" do >>> mock_provider = Factory :mock_provider >>> >> >> ACK and pushed. >> >> Great work! >> >> Thomas > > I think there was a little miscommunication here. I believe this work > was intended to land shortly after the community release (likely > sometime next week). Can you revert this change in the public repo for > now& we'll work with Greg to get it pushed after things settle down > with the community release, etc. > > Thanks in advance, > Mike Hey Mike, Sorry, I didn't know this shouldn't have been pushed yet. I knew Greg didn't have the commit rights so I just went along and let it in. The commit is reverted now. Thomas