#690 Credentials model for provider account (rev.3)

aeolus-conductor seed data (rev 2)

(no subject)

jzigmund＠redhat.com

Thursday, 24 March 2011 Thu, 24 Mar '11

10:38 a.m.

This patchset contains subtasks: #691 - Build credential model to support any provider, #693 - Update the generate_credentials method, #692 - Surface new creds model in 'add account form

Show replies by date

jzigmund＠redhat.com

Thursday, 24 March Thu, 24 Mar

10:38 a.m.

New subject: [PATCH aeolus 1/2] Model of Credentials

From: Jozef Zigmund <jzigmund(a)redhat.com> --- src/app/models/credential.rb | 21 +++++ src/app/models/credential_definition.rb | 24 ++++++ src/app/models/provider_account.rb | 87 ++++++++++++-------- src/app/models/provider_account_observer.rb | 2 +- src/app/models/provider_type.rb | 4 +- src/app/services/data_service_active_record.rb | 8 +- .../migrate/20110308150739_create_credentials.rb | 14 +++ ...20110308160049_create_credential_definitions.rb | 35 ++++++++ ...rm_fields_of_provider_account_to_credentials.rb | 58 +++++++++++++ src/db/seeds.rb | 18 ++++ .../provider_accounts_controller_spec.rb | 11 +-- src/spec/factories/credential.rb | 41 +++++++++ src/spec/factories/credential_definition.rb | 6 ++ src/spec/factories/provider.rb | 2 +- src/spec/factories/provider_account.rb | 29 ++++--- src/spec/factories/provider_type.rb | 4 + src/spec/models/credential_definition_spec.rb | 23 +++++ src/spec/models/credential_spec.rb | 29 +++++++ src/spec/models/instance_spec.rb | 4 +- src/spec/models/pool_family_spec.rb | 2 +- src/spec/models/provider_account_observer_spec.rb | 2 +- src/spec/models/provider_account_spec.rb | 24 +++--- src/spec/models/quota_spec.rb | 1 + .../services/data_service_active_record_spec.rb | 7 +- 24 files changed, 379 insertions(+), 77 deletions(-) create mode 100644 src/app/models/credential.rb create mode 100644 src/app/models/credential_definition.rb create mode 100644 src/db/migrate/20110308150739_create_credentials.rb create mode 100644 src/db/migrate/20110308160049_create_credential_definitions.rb create mode 100644 src/db/migrate/20110309105149_transform_fields_of_provider_account_to_credentials.rb create mode 100644 src/spec/factories/credential.rb create mode 100644 src/spec/factories/credential_definition.rb create mode 100644 src/spec/models/credential_definition_spec.rb create mode 100644 src/spec/models/credential_spec.rb diff --git a/src/app/models/credential.rb b/src/app/models/credential.rb new file mode 100644 index 0000000..39df3b8 --- /dev/null +++ b/src/app/models/credential.rb @@ -0,0 +1,21 @@ +# == Schema Information +# Schema version: 20110309105149 +# +# Table name: credentials +# +# id :integer not null, primary key +# provider_account_id :integer +# value :text +# credential_definition_id :integer not null +# created_at :datetime +# updated_at :datetime +# + +class Credential < ActiveRecord::Base + + belongs_to :provider_account + belongs_to :credential_definition + validates_presence_of :credential_definition_id + validates_presence_of :value + validates_uniqueness_of :credential_definition_id, :scope => :provider_account_id +end diff --git a/src/app/models/credential_definition.rb b/src/app/models/credential_definition.rb new file mode 100644 index 0000000..b809425 --- /dev/null +++ b/src/app/models/credential_definition.rb @@ -0,0 +1,24 @@ +# == Schema Information +# Schema version: 20110309105149 +# +# Table name: credential_definitions +# +# id :integer not null, primary key +# name :string(255) +# label :string(255) +# input_type :string(255) +# provider_type_id :integer +# created_at :datetime +# updated_at :datetime +# + +class CredentialDefinition < ActiveRecord::Base + belongs_to :provider_type + has_many :credentials + validates_presence_of :name + validates_uniqueness_of :name, :scope => :provider_type_id + validates_presence_of :label + validates_presence_of :input_type + validates_presence_of :provider_type_id + CREDENTIAL_DEFINITIONS_ORDER = ["username", "password", "account_id", "x509private", "x509public"] +end diff --git a/src/app/models/provider_account.rb b/src/app/models/provider_account.rb index ce67a51..1f9c13e 100644 --- a/src/app/models/provider_account.rb +++ b/src/app/models/provider_account.rb @@ -1,20 +1,15 @@ # == Schema Information -# Schema version: 20110207110131 +# Schema version: 20110309105149 # # Table name: provider_accounts # -# id :integer not null, primary key -# label :string(255) not null -# username :string(255) not null -# password :string(255) not null -# provider_id :integer not null -# quota_id :integer -# lock_version :integer default(0) -# account_number :string(255) -# x509_cert_priv :text -# x509_cert_pub :text -# created_at :datetime -# updated_at :datetime +# id :integer not null, primary key +# label :string(255) not null +# provider_id :integer not null +# quota_id :integer +# lock_version :integer default(0) +# created_at :datetime +# updated_at :datetime # # @@ -46,7 +41,7 @@ class ProviderAccount < ActiveRecord::Base searchable do text :name, :as => :code_substring - text :username, :as => :code_substring + text(:username, :as => :code_substring) { credentials_hash['username'] } end # Relations @@ -59,6 +54,7 @@ class ProviderAccount < ActiveRecord::Base :order => "permissions.id ASC" has_one :instance_key, :as => :instance_key_owner, :dependent => :destroy + has_many :credentials, :dependent => :destroy # Helpers attr_accessor :x509_cert_priv_file, :x509_cert_pub_file @@ -66,11 +62,8 @@ class ProviderAccount < ActiveRecord::Base # Validations validates_presence_of :provider validates_presence_of :label - validates_presence_of :username - validates_uniqueness_of :username, :scope => :provider_id - validates_presence_of :password - validates_presence_of :account_number,:if => Proc.new{ |account| account.provider.provider_type_id == ProviderType.find_by_codename("ec2").id} - validate :validate_presence_of_x509_certs + validates_uniqueness_of :label + validate :validate_presence_of_credentials validate :validate_credentials # We're using this instead of <tt>validates_presence_of</tt> helper because @@ -118,19 +111,21 @@ class ProviderAccount < ActiveRecord::Base end def read_x509_files + hash = credentials_hash if x509_cert_pub_file.respond_to?(:read) x509_cert_pub_file.rewind # Sometimes the file has to rewind, becuase something already read from it. - self.x509_cert_pub = x509_cert_pub_file.read + hash['x509_cert_pub'] = x509_cert_pub_file.read end if x509_cert_priv_file.respond_to?(:read) x509_cert_priv_file.rewind - self.x509_cert_priv = x509_cert_priv_file.read + hash['x509_cert_priv'] = x509_cert_priv_file.read end + credentials_hash = hash end def connect begin - return DeltaCloud.new(username, password, provider.url) + return DeltaCloud.new(credentials_hash['username'], credentials_hash['password'], provider.url) rescue Exception => e logger.error("Error connecting to framework: #{e.message}") logger.error("Backtrace: #{e.backtrace.join("\n")}") @@ -138,11 +133,6 @@ class ProviderAccount < ActiveRecord::Base end end - def self.find_or_create(account) - a = ProviderAccount.find_by_username_and_provider_id(account["username"], account["provider_id"]) - return a.nil? ? ProviderAccount.new(account) : a - end - def pools pools = [] instances.each do |instance| @@ -151,7 +141,7 @@ class ProviderAccount < ActiveRecord::Base end def name - label.blank? ? username : label + label.blank? ? credentials_hash['username'] : label end # FIXME: for already-mapped accounts, update rather than add new @@ -176,7 +166,7 @@ class ProviderAccount < ActiveRecord::Base end def valid_credentials? - DeltaCloud::valid_credentials?(username, password, provider.url) + DeltaCloud::valid_credentials?(credentials_hash['username'].to_s, credentials_hash['password'].to_s, provider.url) end def build_credentials @@ -193,11 +183,11 @@ class ProviderAccount < ActiveRecord::Base </provider_credentials> EOT node = xml.at_xpath('/provider_credentials/ec2_credentials') - node.at_xpath('./account_number').content = account_number - node.at_xpath('./access_key').content = username - node.at_xpath('./secret_access_key').content = password - node.at_xpath('./certificate').content = x509_cert_pub - node.at_xpath('./key').content = x509_cert_priv + node.at_xpath('./account_number').content = credentials_hash['account_id'] + node.at_xpath('./access_key').content = credentials_hash['username'] + node.at_xpath('./secret_access_key').content = credentials_hash['password'] + node.at_xpath('./certificate').content = credentials_hash['x509public'] + node.at_xpath('./key').content = credentials_hash['x509private'] xml end @@ -206,4 +196,33 @@ EOT return nil unless client && client.feature?(:instances, :authentication_key) client.create_key(:name => "#{self.name}_#{Time.now.to_i}_key_#{self.object_id}") end + + def credentials_hash + @credentials_hash = {} + # Credential.all(:conditions => {:provider_account_id => id}, :include => :credential_definition).each do |cred| + credentials.each do |cred| + @credentials_hash[cred.credential_definition.name] = cred.value + end + @credentials_hash + end + + def credentials_hash=(hash={}) + cred_defs = provider.provider_type.credential_definitions + hash.each do |k,v| + cred_def = cred_defs.detect {|d| d.name == k.to_s} + raise "Key #{k} not found" unless cred_def + unless cred = credentials.detect {|c| c.credential_definition_id == cred_def.id} + cred = Credential.new(:value => v, :provider_account_id => id, :credential_definition_id => cred_def.id) + credentials << cred + end + # we need to handle uploaded files: + cred.value = v.respond_to?(:read) ? v.read : v + end + end + + def all_credentials(prov) + prov.provider_type.credential_definitions.map do |cd| + credentials.detect {|c| c.credential_definition_id == cd.id} || Credential.new(:credential_definition => cd, :value => nil) + end + end end diff --git a/src/app/models/provider_account_observer.rb b/src/app/models/provider_account_observer.rb index 6cd50e2..0f76ee4 100644 --- a/src/app/models/provider_account_observer.rb +++ b/src/app/models/provider_account_observer.rb @@ -15,7 +15,7 @@ class ProviderAccountObserver < ActiveRecord::Observer def create_bucket(account) client = account.connect - bucket_name = "#{account.account_number}-imagefactory-amis" + bucket_name = "#{account.credentials_hash['account_id']}-imagefactory-amis" # TODO (jprovazn): getting particular bucket takes long time (core fetches all # buckets from provider), so we call directly create_bucket, if bucket exists, # exception should be thrown (actually existing bucket is returned - this diff --git a/src/app/models/provider_type.rb b/src/app/models/provider_type.rb index 1f1f58f..8d41e00 100644 --- a/src/app/models/provider_type.rb +++ b/src/app/models/provider_type.rb @@ -1,10 +1,11 @@ # == Schema Information -# Schema version: 20110223132404 +# Schema version: 20110309105149 # # Table name: provider_types # # id :integer not null, primary key # name :string(255) not null +# codename :string(255) not null # ssh_user :string(255) # home_dir :string(255) # build_supported :boolean @@ -16,6 +17,7 @@ class ProviderType < ActiveRecord::Base has_many :providers has_many :images + has_many :credential_definitions, :dependent => :destroy validates_presence_of :name validates_uniqueness_of :name diff --git a/src/app/services/data_service_active_record.rb b/src/app/services/data_service_active_record.rb index 5833baa..0eb6ddd 100644 --- a/src/app/services/data_service_active_record.rb +++ b/src/app/services/data_service_active_record.rb @@ -65,11 +65,11 @@ class DataServiceActiveRecord data_points = [] free_instances = 0 - cloud_accounts = ProviderAccount.find(:all, :conditions => {:provider_id => provider.id}) - cloud_accounts.each do |cloud_account| - quota = cloud_account.quota + provider_accounts = ProviderAccount.find(:all, :conditions => {:provider_id => provider.id}) + provider_accounts.each do |provider_account| + quota = provider_account.quota if quota - data_points << TotalQuotaUsagePoint.new(cloud_account.username, quota.total_instances) + data_points << TotalQuotaUsagePoint.new(provider_account.credentials_hash['username'], quota.total_instances) free_instances += (quota.maximum_total_instances - quota.total_instances) end end diff --git a/src/db/migrate/20110308150739_create_credentials.rb b/src/db/migrate/20110308150739_create_credentials.rb new file mode 100644 index 0000000..579b522 --- /dev/null +++ b/src/db/migrate/20110308150739_create_credentials.rb @@ -0,0 +1,14 @@ +class CreateCredentials < ActiveRecord::Migration + def self.up + create_table :credentials do |t| + t.integer :provider_account_id + t.text :value + t.integer :credential_definition_id, :null => false + t.timestamps + end + end + + def self.down + drop_table :credentials + end +end diff --git a/src/db/migrate/20110308160049_create_credential_definitions.rb b/src/db/migrate/20110308160049_create_credential_definitions.rb new file mode 100644 index 0000000..06b4901 --- /dev/null +++ b/src/db/migrate/20110308160049_create_credential_definitions.rb @@ -0,0 +1,35 @@ +class CreateCredentialDefinitions < ActiveRecord::Migration + def self.up + create_table :credential_definitions do |t| + t.string :name + t.string :label + t.string :input_type + t.integer :provider_type_id + + t.timestamps + end + initialize_table + end + + def self.down + drop_table :credential_definitions + end + + def self.initialize_table + if CredentialDefinition.all.empty? + ProviderType.all.each do |provider_type| + unless provider_type.codename == 'ec2' + CredentialDefinition.create!(:name => 'username', :label => 'API Key', :input_type => 'text', :provider_type_id => provider_type.id) + CredentialDefinition.create!(:name => 'password', :label => 'Secret', :input_type => 'password', :provider_type_id => provider_type.id) + else + #for ec2 provider type + CredentialDefinition.create!(:name => 'username', :label => 'Username', :input_type => 'text', :provider_type_id => provider_type.id) + CredentialDefinition.create!(:name => 'password', :label => 'Password', :input_type => 'password', :provider_type_id => provider_type.id) + CredentialDefinition.create!(:name => 'account_id', :label => 'AWS Account ID', :input_type => 'text', :provider_type_id => provider_type.id) + CredentialDefinition.create!(:name => 'x509private', :label => 'EC2 x509 private key', :input_type => 'file', :provider_type_id => provider_type.id) + CredentialDefinition.create!(:name => 'x509public', :label => 'EC2 x509 public key', :input_type => 'file', :provider_type_id => provider_type.id) + end + end + end + end +end diff --git a/src/db/migrate/20110309105149_transform_fields_of_provider_account_to_credentials.rb b/src/db/migrate/20110309105149_transform_fields_of_provider_account_to_credentials.rb new file mode 100644 index 0000000..c7364ca --- /dev/null +++ b/src/db/migrate/20110309105149_transform_fields_of_provider_account_to_credentials.rb @@ -0,0 +1,58 @@ +class TransformFieldsOfProviderAccountToCredentials < ActiveRecord::Migration + def self.up + transform + remove_column :provider_accounts, :username + remove_column :provider_accounts, :password + remove_column :provider_accounts, :account_number + remove_column :provider_accounts, :x509_cert_priv + remove_column :provider_accounts, :x509_cert_pub + + end + + def self.down + add_column :provider_accounts, :username, :string + add_column :provider_accounts, :password, :string + add_column :provider_accounts, :account_number, :string + add_column :provider_accounts, :x509_cert_priv, :text + add_column :provider_accounts, :x509_cert_pub, :text + transform_back + end + + def self.transform + unless ProviderAccount.all.empty? + ProviderAccount.all.each do |account| + Credential.create!(:provider_account_id => account.id, :credential_definition_id => CredentialDefinition.find_by_name('username',:conditions => {:provider_type_id => account.provider.provider_type.id}).id, :value => account.username) + Credential.create!(:provider_account_id => account.id, :credential_definition_id => CredentialDefinition.find_by_name('password',:conditions => {:provider_type_id => account.provider.provider_type.id}).id, :value => account.password) + + # transform more attributes for ec2 + if account.provider.provider_type.codename == 'ec2' + Credential.create!(:provider_account_id => account.id, :credential_definition_id => CredentialDefinition.find_by_name('account_id',:conditions => {:provider_type_id => account.provider.provider_type.id}).id, :value => account.account_number) + Credential.create!(:provider_account_id => account.id, :credential_definition_id => CredentialDefinition.find_by_name('x509private',:conditions => {:provider_type_id => account.provider.provider_type.id}).id, :value => account.x509_cert_priv) + Credential.create!(:provider_account_id => account.id, :credential_definition_id => CredentialDefinition.find_by_name('x509public',:conditions => {:provider_type_id => account.provider.provider_type.id}).id, :value => account.x509_cert_pub) + end + end + end + end + + def self.transform_back + unless Credential.all.empty? + Credential.all.each do |credential| + if credential.credential_definition.name == 'username' + credential.provider_account.update_attribute(:username, credential.value) + elsif credential.credential_definition.name == 'password' + credential.provider_account.update_attribute(:password, credential.value) + end + if credential.credential_definition.provider_type.codename == 'ec2' + if credential.credential_definition.name == 'account_id' + credential.provider_account.update_attribute(:account_number, credential.value) + elsif credential.credential_definition.name == 'x509private' + credential.provider_account.update_attribute(:x509_cert_priv, credential.value) + elsif credential.credential_definition.name == 'x509public' + credential.provider_account.update_attribute(:x509_cert_pub, credential.value) + end + end + end + end + end + +end diff --git a/src/db/seeds.rb b/src/db/seeds.rb index cf23e60..33c78ed 100644 --- a/src/db/seeds.rb +++ b/src/db/seeds.rb @@ -112,4 +112,22 @@ if ProviderType.all.empty? ProviderType.create!(:name => "Rackspace", :codename =>"rackspace") ProviderType.create!(:name => "RHEV-M", :codename =>"rhevm") ProviderType.create!(:name => "OpenNebula", :codename =>"opennebula") +end + +# fill table CredentialDefinitions by default values +if CredentialDefinition.all.empty? + ProviderType.all.each do |provider_type| + unless provider_type.codename == 'ec2' + CredentialDefinition.create!(:name => 'username', :label => 'API Key', :input_type => 'text', :provider_type_id => provider_type.id) + CredentialDefinition.create!(:name => 'password', :label => 'Secret', :input_type => 'password', :provider_type_id => provider_type.id) + end end + + #for ec2 provider type + ec2 = ProviderType.find_by_codename 'ec2' + CredentialDefinition.create!(:name => 'username', :label => 'Username', :input_type => 'text', :provider_type_id => ec2.id) + CredentialDefinition.create!(:name => 'password', :label => 'Password', :input_type => 'password', :provider_type_id => ec2.id) + CredentialDefinition.create!(:name => 'account_id', :label => 'AWS Account ID', :input_type => 'text', :provider_type_id => ec2.id) + CredentialDefinition.create!(:name => 'x509private', :label => 'EC2 x509 private key', :input_type => 'file', :provider_type_id => ec2.id) + CredentialDefinition.create!(:name => 'x509public', :label => 'EC2 x509 public key', :input_type => 'file', :provider_type_id => ec2.id) +end diff --git a/src/spec/controllers/provider_accounts_controller_spec.rb b/src/spec/controllers/provider_accounts_controller_spec.rb index 567538b..1e27bfa 100644 --- a/src/spec/controllers/provider_accounts_controller_spec.rb +++ b/src/spec/controllers/provider_accounts_controller_spec.rb @@ -23,6 +23,7 @@ describe Admin::ProviderAccountsController do it "allows test account validity on create when passing test_account param" do UserSession.create(@admin) + @provider_account.credentials_hash = {} post :create, :provider_account => {:provider_id => @provider.id}, :test_account => true response.should be_success response.should render_template("new") @@ -34,7 +35,7 @@ describe Admin::ProviderAccountsController do post :create, :provider_account => {:provider_id => @provider.id} response.should be_success response.should render_template("new") - response.flash[:error].should == "The entered credential information is incorrect" + response.flash[:error].should == "Credentials are invalid!" end it "should permit users with account modify permission to access edit cloud account interface" do @@ -46,15 +47,13 @@ describe Admin::ProviderAccountsController do it "should allow users with account modify password to update a cloud account" do UserSession.create(@admin) - - @provider_account.password = "foobar" + @provider_account.credentials_hash = {: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 => { :password => 'mockpassword' } + post :update, :id => @provider_account.id, :provider_account => { :credentials_hash => {:password => 'mockpassword'} } response.should redirect_to admin_provider_account_path(@provider_account) - ProviderAccount.find((a)provider_account.id).password.should == "mockpassword" + ProviderAccount.find((a)provider_account.id).credentials_hash['password'].should == "mockpassword" end it "should allow users with account modify permission to delete a cloud account" do diff --git a/src/spec/factories/credential.rb b/src/spec/factories/credential.rb new file mode 100644 index 0000000..8cde3f8 --- /dev/null +++ b/src/spec/factories/credential.rb @@ -0,0 +1,41 @@ +Factory.define :credential do |c| + c.association :credential_definition + c.sequence(:value) {|n| "value#{n}"} +end + +# EC2 credentials +Factory.define :ec2_username_credential, :parent => :credential do |c| + c.value "mockuser" + c.credential_definition { CredentialDefinition.find_by_name('username',:conditions => {:provider_type_id => ProviderType.find_by_codename('ec2')})} +end + +Factory.define :ec2_password_credential, :parent => :credential do |c| + c.value "mockpassword" + c.credential_definition { CredentialDefinition.find_by_name('password',:conditions => {:provider_type_id => ProviderType.find_by_codename('ec2')})} +end + +Factory.define :ec2_account_id_credential, :parent => :credential do |c| + c.value "3141" + c.credential_definition { CredentialDefinition.find_by_name('account_id',:conditions => {:provider_type_id => ProviderType.find_by_codename('ec2')})} +end + +Factory.define :ec2_x509private_credential, :parent => :credential do |c| + c.value "x509 private key" + c.credential_definition { CredentialDefinition.find_by_name('x509private',:conditions => {:provider_type_id => ProviderType.find_by_codename('ec2')})} +end + +Factory.define :ec2_x509public_credential, :parent => :credential do |c| + c.value "x509 public key" + c.credential_definition { CredentialDefinition.find_by_name('x509public',:conditions => {:provider_type_id => ProviderType.find_by_codename('ec2')})} +end + +#Mock & Others credentials +Factory.define :username_credential, :parent => :credential do |c| + c.value "mockuser" + c.credential_definition { CredentialDefinition.find_by_name('username',:conditions => {:provider_type_id => ProviderType.find_by_codename('mock')})} +end + +Factory.define :password_credential, :parent => :credential do |c| + c.value "mockpassword" + c.credential_definition { CredentialDefinition.find_by_name('password',:conditions => {:provider_type_id => ProviderType.find_by_codename('mock')})} +end diff --git a/src/spec/factories/credential_definition.rb b/src/spec/factories/credential_definition.rb new file mode 100644 index 0000000..31424d6 --- /dev/null +++ b/src/spec/factories/credential_definition.rb @@ -0,0 +1,6 @@ +Factory.define :credential_definition do |f| + f.sequence(:name) { |n| "field#{n}" } + f.sequence(:label) { |n| "field#{n}" } + f.input_type 'text' + f.association :provider_type +end diff --git a/src/spec/factories/provider.rb b/src/spec/factories/provider.rb index 34f3011..587bc59 100644 --- a/src/spec/factories/provider.rb +++ b/src/spec/factories/provider.rb @@ -5,7 +5,7 @@ Factory.define :provider do |p| end Factory.define :mock_provider, :parent => :provider do |p| - p.provider_type { ProviderType.find_by_codename("mock") } + p.provider_type {ProviderType.find_by_codename("mock")} p.url 'http://localhost:3001/api' p.hardware_profiles { |hp| [hp.association(:mock_hwp1), hp.association(:mock_hwp2)] } p.after_create { |p| p.realms << Factory(:realm1, :provider => p) << Factory(:realm2, :provider => p) } diff --git a/src/spec/factories/provider_account.rb b/src/spec/factories/provider_account.rb index bad1567..d3944bd 100644 --- a/src/spec/factories/provider_account.rb +++ b/src/spec/factories/provider_account.rb @@ -1,23 +1,28 @@ Factory.define :provider_account do |f| - f.sequence(:username) { |n| "testUser#{n}" } - f.password "testPassword" f.sequence(:label) { |n| "test label#{n}" } - f.account_number "3141" - f.x509_cert_priv "x509 private key" - f.x509_cert_pub "x509 public key" f.association :provider f.association :quota - f.after_build {|acc| acc.stub!(:generate_auth_key).and_return(nil) if acc.respond_to?(:stub!)} + f.after_build do |acc| + acc.stub!(:generate_auth_key).and_return(nil) if acc.respond_to?(:stub!) + end end Factory.define :mock_provider_account, :parent => :provider_account do |f| - f.username "mockuser" - f.password "mockpassword" - f.provider { |p| p.association(:mock_provider) } + f.association :provider, :factory => :mock_provider + f.after_build do |acc| + acc.credentials << Factory.build(:username_credential) + acc.credentials << Factory.build(:password_credential) + end end Factory.define :ec2_provider_account, :parent => :provider_account do |f| - f.username "mockuser" - f.password "mockpassword" - f.provider { |p| p.association(:ec2_provider) } + f.association :provider, :factory => :ec2_provider + f.after_build do |acc| + acc.credentials << Factory.build(:ec2_username_credential) + acc.credentials << Factory.build(:ec2_password_credential) + acc.credentials << Factory.build(:ec2_account_id_credential) + acc.credentials << Factory.build(:ec2_x509private_credential) + acc.credentials << Factory.build(:ec2_x509public_credential) + end + end diff --git a/src/spec/factories/provider_type.rb b/src/spec/factories/provider_type.rb index 0fed927..2e73f4a 100644 --- a/src/spec/factories/provider_type.rb +++ b/src/spec/factories/provider_type.rb @@ -1,10 +1,14 @@ Factory.define :provider_type do |p| + p.sequence(:name) { |n| "name#{n}" } + p.sequence(:codename) { |n| "codename#{n}" } end Factory.define :mock_provider_type, :parent => :provider_type do |p| p.name 'Mock' + p.codename 'mock' end Factory.define :ec2_provider_type, :parent => :provider_type do |p| p.name 'Amazon EC2' + p.codename 'ec2' end diff --git a/src/spec/models/credential_definition_spec.rb b/src/spec/models/credential_definition_spec.rb new file mode 100644 index 0000000..9b39e21 --- /dev/null +++ b/src/spec/models/credential_definition_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +context CredentialDefinition do + before(:each) do + @cred_def = Factory.build(:credential_definition) + end + + it "default factory object should be valid" do + @cred_def.should be_valid + end + + + it "should not be valid without name" do + @cred_def.name = nil + @cred_def.should_not be_valid + end + + it "should not be valid without label" do + @cred_def.label = nil + @cred_def.should_not be_valid + end + +end diff --git a/src/spec/models/credential_spec.rb b/src/spec/models/credential_spec.rb new file mode 100644 index 0000000..4a1af05 --- /dev/null +++ b/src/spec/models/credential_spec.rb @@ -0,0 +1,29 @@ +require "spec_helper" + +describe Credential do + before(:each) do + @credential = Factory.build(:credential) + end + + it "default factory object is valid" do + @credential.should be_valid + end + + it "should not be valid without value" do + @credential.value = nil + @credential.should_not be_valid + end + + it "should not be valid without assigned credential definition" do + @credential.credential_definition_id = nil + @credential.should_not be_valid + end + + it "should not be valid without unique credential definition" do + @credential.save! + @second_credential = Factory.build(:credential, + :credential_definition_id => @credential.credential_definition_id, + :provider_account_id => @credential.provider_account_id) + @second_credential.should_not be_valid + end +end diff --git a/src/spec/models/instance_spec.rb b/src/spec/models/instance_spec.rb index fd5d00f..8d279de 100644 --- a/src/spec/models/instance_spec.rb +++ b/src/spec/models/instance_spec.rb @@ -132,9 +132,7 @@ describe Instance do it "should return empty list of instance actions when connect to provider fails" do provider = Factory.build(:mock_provider2) - cloud_account = Factory.build(:provider_account, :provider => provider, - :username => 'john doe', - :password => 'asdf') + cloud_account = Factory.build(:provider_account, :provider => provider) cloud_account.stub!(:connect).and_return(nil) cloud_account.stub!(:valid_credentials?).and_return(true) instance = Factory.create(:instance, :provider_account => cloud_account) diff --git a/src/spec/models/pool_family_spec.rb b/src/spec/models/pool_family_spec.rb index 68dfb42..3a40201 100644 --- a/src/spec/models/pool_family_spec.rb +++ b/src/spec/models/pool_family_spec.rb @@ -5,7 +5,7 @@ describe PoolFamily do before(:each) do @pool = Factory :pool @pool_family = @pool.pool_family - @provider_account = Factory :mock_provider_account + @provider_account = Factory.build :mock_provider_account @provider_account.pool_families << @pool_family @provider_account.save! end diff --git a/src/spec/models/provider_account_observer_spec.rb b/src/spec/models/provider_account_observer_spec.rb index cc8b210..6b7d62a 100644 --- a/src/spec/models/provider_account_observer_spec.rb +++ b/src/spec/models/provider_account_observer_spec.rb @@ -15,7 +15,7 @@ describe ProviderAccountObserver do provider_account = Factory.build :ec2_provider_account provider_account.stub!(:connect).and_return(@client) provider_account.stub!(:generate_auth_key).and_return(@key) - provider_account.save + provider_account.save! provider_account.instance_key.should_not == nil provider_account.instance_key.pem == "PEM" provider_account.instance_key.id == "1_user" diff --git a/src/spec/models/provider_account_spec.rb b/src/spec/models/provider_account_spec.rb index 943bee9..1c9ca5c 100644 --- a/src/spec/models/provider_account_spec.rb +++ b/src/spec/models/provider_account_spec.rb @@ -3,14 +3,13 @@ require 'spec_helper' describe ProviderAccount do fixtures :all before(:each) do - @provider_account = Factory :mock_provider_account + @provider_account = Factory.build :mock_provider_account end it "should not be destroyable if it has instances" do @provider_account.instances << Instance.new @provider_account.destroyable?.should be_false @provider_account.destroy.should be_false - @provider_account.instances.clear @provider_account.destroyable?.should be_true @provider_account.destroy.equal?((a)provider_account).should be_true @@ -20,7 +19,8 @@ describe ProviderAccount do it "should check the validitiy of the cloud account login credentials" do mock_provider = Factory :mock_provider - invalid_provider_account = Factory.build(:provider_account, :username => "wrong_username", :password => "wrong_password", :provider => mock_provider) + invalid_provider_account = Factory(:mock_provider_account, :provider => mock_provider) + invalid_provider_account.credentials_hash = {'username' => "wrong_username", 'password' => "wrong_password"} invalid_provider_account.should_not be_valid valid_provider_account = Factory.build(:mock_provider_account, :provider => mock_provider) @@ -28,7 +28,8 @@ describe ProviderAccount do end it "should fail to create a cloud account if the provider credentials are invalid" do - provider_account = Factory.build(:mock_provider_account, :password => "wrong_password") + provider_account = Factory.build(:mock_provider_account) + provider_account.credentials_hash = {'password' => "wrong_password"} provider_account.save.should == false end @@ -69,13 +70,14 @@ describe ProviderAccount do </ec2_credentials> </provider_credentials> EOT - provider_account = Factory.build(:mock_provider_account, - :username => 'user', - :password => 'pass', - :account_number => '1234', - :x509_cert_priv => 'priv_key', - :x509_cert_pub => 'cert' - ) + provider_account = Factory.build(:ec2_provider_account) + provider_account.credentials_hash = { + 'username' => 'user', + 'password' => 'pass', + 'account_id' => '1234', + 'x509private' => 'priv_key', + 'x509public' => 'cert' + } provider_account.build_credentials.to_s.should eql(expected_xml) end end diff --git a/src/spec/models/quota_spec.rb b/src/spec/models/quota_spec.rb index 960b417..8455619 100644 --- a/src/spec/models/quota_spec.rb +++ b/src/spec/models/quota_spec.rb @@ -28,6 +28,7 @@ describe Quota do @pool.quota = Factory :unlimited_quota @pool.save! + @provider_account = Factory(:mock_provider_account, :quota_id => @provider_account_quota.id) @provider_account.quota = Factory :unlimited_quota @provider_account.save! diff --git a/src/spec/services/data_service_active_record_spec.rb b/src/spec/services/data_service_active_record_spec.rb index efe75f5..d108e01 100644 --- a/src/spec/services/data_service_active_record_spec.rb +++ b/src/spec/services/data_service_active_record_spec.rb @@ -12,7 +12,8 @@ describe DataServiceActiveRecord do free = 0 for i in 0..2 quota = Factory(:quota, :maximum_total_instances => data[i][0], :total_instances => data[i][1]) - provider_account = Factory.build(:provider_account, :provider => provider, :username => "username" + i.to_s, :quota => quota) + provider_account = Factory.build(:provider_account, :provider => provider, :quota => quota) + provider_account.credentials_hash = {:username => "username" + i.to_s, :password => 'mockpassword'} provider_account.stub!(:valid_credentials?).and_return(true) provider_account.save! @@ -41,6 +42,7 @@ describe DataServiceActiveRecord do :total_instances => 20) provider_account = Factory.build(:provider_account, :provider => provider, :quota => quota) + provider_account.credentials_hash = {:username => 'test', :password =>'test'} provider_account.stub!(:valid_credentials?).and_return(true) provider_account.save! @@ -126,7 +128,8 @@ describe DataServiceActiveRecord do provider_accounts = [] expected_averages.each do |expected_average| - provider_account = Factory.build(:provider_account, :provider => provider, :username => "username" + expected_average[0].to_s) + provider_account = Factory.build(:provider_account, :provider => provider) + provider_account.credentials_hash = { :username => "username" + expected_average[0].to_s, :password => 'mockpassword' } provider_account.stub!(:valid_credentials?).and_return(true) provider_account.save! -- 1.7.4

jzigmund＠redhat.com

10:38 a.m.

New subject: [PATCH aeolus 2/2] Surface new creds model in 'add account form'

From: Jiri Tomasek <jtomasek(a)redhat.com> --- .../admin/provider_accounts_controller.rb | 17 +++------ src/app/helpers/admin/provider_accounts_helper.rb | 10 ----- src/app/models/provider_account.rb | 31 ++++------------ src/app/views/admin/provider_accounts/_ec2.haml | 39 -------------------- src/app/views/admin/provider_accounts/_gogrid.haml | 16 -------- src/app/views/admin/provider_accounts/_list.haml | 2 +- src/app/views/admin/provider_accounts/_mock.haml | 16 -------- .../views/admin/provider_accounts/_opennebula.haml | 16 -------- .../views/admin/provider_accounts/_properties.haml | 2 +- .../admin/provider_accounts/_provider_form.haml | 33 +++++++++++++++++ .../provider_accounts/_provider_selection.haml | 7 +++- .../views/admin/provider_accounts/_rackspace.haml | 16 -------- src/app/views/admin/provider_accounts/new.haml | 4 +- src/features/provider_account.feature | 14 ++++---- 14 files changed, 63 insertions(+), 160 deletions(-) delete mode 100644 src/app/helpers/admin/provider_accounts_helper.rb delete mode 100644 src/app/views/admin/provider_accounts/_ec2.haml delete mode 100644 src/app/views/admin/provider_accounts/_gogrid.haml delete mode 100644 src/app/views/admin/provider_accounts/_mock.haml delete mode 100644 src/app/views/admin/provider_accounts/_opennebula.haml create mode 100644 src/app/views/admin/provider_accounts/_provider_form.haml delete mode 100644 src/app/views/admin/provider_accounts/_rackspace.haml diff --git a/src/app/controllers/admin/provider_accounts_controller.rb b/src/app/controllers/admin/provider_accounts_controller.rb index ceffe17..49fced8 100644 --- a/src/app/controllers/admin/provider_accounts_controller.rb +++ b/src/app/controllers/admin/provider_accounts_controller.rb @@ -44,7 +44,7 @@ class Admin::ProviderAccountsController < ApplicationController if @providers.empty? flash[:error] = "You don't have any provider yet. Please create one!" else - @selected_provider = @providers.first unless @providers.blank? + @selected_provider = @providers.first unless @providers.blank? end end @@ -66,13 +66,7 @@ class Admin::ProviderAccountsController < ApplicationController @provider_account.quota.set_maximum_running_instances(limit) if @provider_account.invalid? - if not @provider_account.valid_credentials? - flash.now[:error] = "The entered credential information is incorrect" - elsif @provider_account.errors.on(:username) - flash.now[:error] = "The access key '#{params[:provider_account][:username]}' has already been taken." - else - flash.now[:error] = "You must fill in all the required fields" - end + flash[:error] = "Credentials are invalid!" render :action => 'new' and return end @@ -174,10 +168,11 @@ class Admin::ProviderAccountsController < ApplicationController end def set_view_vars + #FIXME need to include atributes from credentials, credential_definitions and provider_type in load_accounts query to make it work @header = [ - { :name => "Name", :sort_attr => :name }, - { :name => "Username", :sort_attr => :username}, - { :name => "Provider Type", :sort_attr => :provider_type } + { :name => "Name", :sortable => false }, + { :name => "Username", :sortable => false}, + { :name => "Provider Type", :sortable => false } ] @url_params = params end diff --git a/src/app/helpers/admin/provider_accounts_helper.rb b/src/app/helpers/admin/provider_accounts_helper.rb deleted file mode 100644 index a6bf5ef..0000000 --- a/src/app/helpers/admin/provider_accounts_helper.rb +++ /dev/null @@ -1,10 +0,0 @@ -module Admin::ProviderAccountsHelper - - def display_provider_account_login_form(provider_type) - unless provider_type.codename.nil? - render :partial => provider_type.codename - else - flash.now[:warning] = "You don't have any provider yet" - end - end -end diff --git a/src/app/models/provider_account.rb b/src/app/models/provider_account.rb index 1f9c13e..a53b4ae 100644 --- a/src/app/models/provider_account.rb +++ b/src/app/models/provider_account.rb @@ -54,7 +54,9 @@ class ProviderAccount < ActiveRecord::Base :order => "permissions.id ASC" has_one :instance_key, :as => :instance_key_owner, :dependent => :destroy - has_many :credentials, :dependent => :destroy + # 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 # Helpers attr_accessor :x509_cert_priv_file, :x509_cert_pub_file @@ -66,24 +68,20 @@ class ProviderAccount < ActiveRecord::Base validate :validate_presence_of_credentials validate :validate_credentials - # We're using this instead of <tt>validates_presence_of</tt> helper because - # we want to show errors on different attributes ending with '_file'. - def validate_presence_of_x509_certs - if self.provider.provider_type_id == ProviderType.find_by_codename("ec2").id - errors.add(:x509_cert_pub_file, "can't be blank") if x509_cert_pub.blank? - errors.add(:x509_cert_priv_file, "can't be blank") if x509_cert_priv.blank? + def validate_presence_of_credentials + provider.provider_type.credential_definitions.each do |cd| + errors.add(:base, "#{cd.label} can't be blank") if credentials_hash[cd.name].blank? end end def validate_credentials unless valid_credentials? - errors.add(:base, "Login Credentials are Invalid for this Provider") + errors.add(:base, "Login Credenials are Invalid for this Provider") end end # Hooks before_destroy :destroyable? - before_validation :read_x509_files def object_list super << provider @@ -110,19 +108,6 @@ class ProviderAccount < ActiveRecord::Base instances.empty? end - def read_x509_files - hash = credentials_hash - if x509_cert_pub_file.respond_to?(:read) - x509_cert_pub_file.rewind # Sometimes the file has to rewind, becuase something already read from it. - hash['x509_cert_pub'] = x509_cert_pub_file.read - end - if x509_cert_priv_file.respond_to?(:read) - x509_cert_priv_file.rewind - hash['x509_cert_priv'] = x509_cert_priv_file.read - end - credentials_hash = hash - end - def connect begin return DeltaCloud.new(credentials_hash['username'], credentials_hash['password'], provider.url) @@ -212,7 +197,7 @@ EOT cred_def = cred_defs.detect {|d| d.name == k.to_s} raise "Key #{k} not found" unless cred_def unless cred = credentials.detect {|c| c.credential_definition_id == cred_def.id} - cred = Credential.new(:value => v, :provider_account_id => id, :credential_definition_id => cred_def.id) + cred = Credential.new(:provider_account_id => id, :credential_definition_id => cred_def.id) credentials << cred end # we need to handle uploaded files: diff --git a/src/app/views/admin/provider_accounts/_ec2.haml b/src/app/views/admin/provider_accounts/_ec2.haml deleted file mode 100644 index 43c4078..0000000 --- a/src/app/views/admin/provider_accounts/_ec2.haml +++ /dev/null @@ -1,39 +0,0 @@ -%fieldset.nomargin.clearfix - = label_tag "Account name:" - = text_field :provider_account, :label - = label_tag "Username" - = text_field :provider_account, :username - = label_tag "Password" - = password_field :provider_account, :password - = label_tag "Quota" - = text_field :quota, :maximum_running_instances, :title => t('provider_accounts.form.quota_instances'), :value => @quota.maximum_running_instances || "unlimited", :id => "quota_instances", :class => "grid_3 omega" -%fieldset.nomargin.clearfix - .grid_3.prefix_10.alpha.omega - ( - %button.linkbutton.nospace{ :type => 'button', :onclick => "set_unlimited_quota(\"quota_instances\");" }<> - = t('provider_accounts.form.unlimited_quota') - ) - -%fieldset.clearfix.nomargin - %label.grid_4.la.alpha - = t('provider_accounts.form.account_number') - %span.required * - = text_field :provider_account, :account_number - %label.grid_3.la - = t('provider_accounts.form.account_private_cert') - %span.required * - = file_field :provider_account, :x509_cert_priv_file, :title => t('provider_accounts.form.account_private_cert') - %label.grid_3.la - = t('provider_accounts.form.account_public_cert') - %span.required * - = file_field :provider_account, :x509_cert_pub_file, :title => t('provider_accounts.form.account_public_cert') - .grid_3.omega - ( - %button.linkbutton.nospace{ :type => 'submit', :value => t('provider_accounts.form.test_account'), :name => 'test_account', :id => 'test_account' }<> - = t('provider_accounts.form.test_account') - ) - -:javascript - function set_unlimited_quota(elem_id) { - $("#" + elem_id)[0].value = "unlimited"; - } diff --git a/src/app/views/admin/provider_accounts/_gogrid.haml b/src/app/views/admin/provider_accounts/_gogrid.haml deleted file mode 100644 index 421768e..0000000 --- a/src/app/views/admin/provider_accounts/_gogrid.haml +++ /dev/null @@ -1,16 +0,0 @@ -%fieldset.clear - = label_tag "Account name:" - = text_field :provider_account, :label -%fieldset.clear - = label_tag :username, "API Key:" - = text_field :provider_account,:username -%fieldset.clear - = label_tag :password, "Secret:" - = password_field :provider_account,:password - = label_tag "Quota" - = text_field :quota, :maximum_running_instances, :title => t('provider_accounts.form.quota_instances'), :value => @quota.maximum_running_instances || "unlimited", :id => "quota_instances", :class => "grid_3 omega" -.grid_3.omega - ( - %button.linkbutton.nospace{ :type => 'submit', :value => t('provider_accounts.form.test_account'), :name => 'test_account', :id => 'test_account' }<> - = t('provider_accounts.form.test_account') - ) diff --git a/src/app/views/admin/provider_accounts/_list.haml b/src/app/views/admin/provider_accounts/_list.haml index 687c32e..b431f9b 100644 --- a/src/app/views/admin/provider_accounts/_list.haml +++ b/src/app/views/admin/provider_accounts/_list.haml @@ -15,6 +15,6 @@ %input{:name => 'accounts_selected[]', :type => 'checkbox', :value => account.id, :id => "account_checkbox_#{account.id}", :checked => selected } = link_to account.name, admin_provider_account_path(account) %td - =account.username + =account.credentials_hash['username'] %td =account.provider.provider_type.name diff --git a/src/app/views/admin/provider_accounts/_mock.haml b/src/app/views/admin/provider_accounts/_mock.haml deleted file mode 100644 index 421768e..0000000 --- a/src/app/views/admin/provider_accounts/_mock.haml +++ /dev/null @@ -1,16 +0,0 @@ -%fieldset.clear - = label_tag "Account name:" - = text_field :provider_account, :label -%fieldset.clear - = label_tag :username, "API Key:" - = text_field :provider_account,:username -%fieldset.clear - = label_tag :password, "Secret:" - = password_field :provider_account,:password - = label_tag "Quota" - = text_field :quota, :maximum_running_instances, :title => t('provider_accounts.form.quota_instances'), :value => @quota.maximum_running_instances || "unlimited", :id => "quota_instances", :class => "grid_3 omega" -.grid_3.omega - ( - %button.linkbutton.nospace{ :type => 'submit', :value => t('provider_accounts.form.test_account'), :name => 'test_account', :id => 'test_account' }<> - = t('provider_accounts.form.test_account') - ) diff --git a/src/app/views/admin/provider_accounts/_opennebula.haml b/src/app/views/admin/provider_accounts/_opennebula.haml deleted file mode 100644 index abcf8a2..0000000 --- a/src/app/views/admin/provider_accounts/_opennebula.haml +++ /dev/null @@ -1,16 +0,0 @@ -%fieldset.clear - = label_tag "Account name:" - = text_field :provider_account, :label -%fieldset.clear - = label_tag :username, "API Key:" - = text_field :provider_account, :username -%fieldset.clear - = label_tag :password, "Secret:" - = password_field :provider_account, :password - = label_tag "Quota" - = text_field :quota, :maximum_running_instances, :title => t('provider_accounts.form.quota_instances'), :value => @quota.maximum_running_instances || "unlimited", :id => "quota_instances", :class => "grid_3 omega" -.grid_3.omega - ( - %button.linkbutton.nospace{ :type => 'submit', :value => t('provider_accounts.form.test_account'), :name => 'test_account', :id => 'test_account' }<> - = t('provider_accounts.form.test_account') - ) diff --git a/src/app/views/admin/provider_accounts/_properties.haml b/src/app/views/admin/provider_accounts/_properties.haml index a3e127d..27789a1 100644 --- a/src/app/views/admin/provider_accounts/_properties.haml +++ b/src/app/views/admin/provider_accounts/_properties.haml @@ -8,6 +8,6 @@ %p %label Account ID: - = @account.account_number + = @account.credentials_hash['account_number'] = link_to "Edit", edit_admin_provider_account_path(@account), { :class => 'button' } = link_to "Test", admin_provider_account_path(@account, {:test_account => true}), { :class => 'button' } diff --git a/src/app/views/admin/provider_accounts/_provider_form.haml b/src/app/views/admin/provider_accounts/_provider_form.haml new file mode 100644 index 0000000..d4c1462 --- /dev/null +++ b/src/app/views/admin/provider_accounts/_provider_form.haml @@ -0,0 +1,33 @@ +%fieldset.clear + = label :provider_account, "Account name:" + = text_field :provider_account, :label +- CredentialDefinition::CREDENTIAL_DEFINITIONS_ORDER.each do |cred_order| + - @provider_account.all_credentials((a)selected_provider).each do |cred| + - if cred_order == cred.credential_definition.name + %fieldset.clear + = label_tag cred.credential_definition.label + - if cred.credential_definition.input_type == "password" + = password_field_tag "provider_account[credentials_hash][#{cred.credential_definition.name}]", cred.value + - elsif cred.credential_definition.input_type == "file" + = file_field_tag "provider_account[credentials_hash][#{cred.credential_definition.name}]", :value => cred.value + - else + = text_field_tag "provider_account[credentials_hash][#{cred.credential_definition.name}]", cred.value + += label_tag "Quota" += text_field :quota, :maximum_running_instances, :title => t('provider_accounts.form.quota_instances'), :value => @quota.maximum_running_instances || "unlimited", :id => "quota_instances", :class => "grid_3 omega" +( +%button.linkbutton.nospace{ :type => 'button', :onclick => "set_unlimited_quota(\"quota_instances\");" }<> + = t('provider_accounts.form.unlimited_quota') +) +%fieldset.clear + +.grid_3.omega + ( + %button.linkbutton.nospace{ :type => 'submit', :value => t('provider_accounts.form.test_account'), :name => 'test_account', :id => 'test_account' }<> + = t('provider_accounts.form.test_account') + ) + +:javascript + function set_unlimited_quota(elem_id) { + $("#" + elem_id)[0].value = "unlimited"; + } diff --git a/src/app/views/admin/provider_accounts/_provider_selection.haml b/src/app/views/admin/provider_accounts/_provider_selection.haml index a6ffb45..2095fde 100644 --- a/src/app/views/admin/provider_accounts/_provider_selection.haml +++ b/src/app/views/admin/provider_accounts/_provider_selection.haml @@ -1,10 +1,13 @@ %fieldset#provider_type.clear = label_tag "Provider: " - = select(:provider_account ,:provider_id, options_for_select((a)providers.map{ |p| [p.name, p.id] }, :selected => @selected_provider.id)) + = select(:provider_account, :provider_id, options_for_select((a)providers.map{ |p| [p.name, p.id] }, :selected => @selected_provider.id)) = restful_submit_tag "Choose", 'set_selected_provider',set_selected_provider_admin_provider_accounts_path, 'GET', :class => 'button' = label_tag "Cloud type: " = @selected_provider.provider_type.name - = display_provider_account_login_form((a)selected_provider.provider_type) + - unless @selected_provider.provider_type.codename.nil? + = render :partial => "provider_form", :locals => { :provider_type => @selected_provider.provider_type} + - else + = flash.now[:warning] = "You don't have any provider yet" :javascript $(document).ready(function(){ diff --git a/src/app/views/admin/provider_accounts/_rackspace.haml b/src/app/views/admin/provider_accounts/_rackspace.haml deleted file mode 100644 index bc99d32..0000000 --- a/src/app/views/admin/provider_accounts/_rackspace.haml +++ /dev/null @@ -1,16 +0,0 @@ -%fieldset.clear - = label_tag "Account name:" - = text_field :provider_account, :label -%fieldset.clear - = label_tag:username, "API Key:" - = text_field :provider_account, :username -%fieldset.clear - = label_tag :password, "Secret:" - = password_field :provider_account, :password - = label_tag "Quota" - = text_field :quota, :maximum_running_instances, :title => t('provider_accounts.form.quota_instances'), :value => @quota.maximum_running_instances || "unlimited", :id => "quota_instances", :class => "grid_3 omega" -.grid_3.omega - ( - %button.linkbutton.nospace{ :type => 'submit', :value => t('provider_accounts.form.test_account'), :name => 'test_account', :id => 'test_account' }<> - = t('provider_accounts.form.test_account') - ) diff --git a/src/app/views/admin/provider_accounts/new.haml b/src/app/views/admin/provider_accounts/new.haml index a0db904..f1a5e2a 100644 --- a/src/app/views/admin/provider_accounts/new.haml +++ b/src/app/views/admin/provider_accounts/new.haml @@ -6,11 +6,11 @@ Provider %fieldset.clear - unless @providers.empty? - - form_tag(admin_provider_accounts_path, :multipart => true) do + - form_for(@provider_account, :url => admin_provider_accounts_path, :html => {:multipart => true}) do |f| = render :partial => 'provider_selection' %fieldset.clearfix .grid_13.alpha.omega - = submit_tag t(:add), :class => "ra nomargin dialogbutton" + = f.submit t(:add), :class => "ra nomargin dialogbutton" %section %p.requirement %span.required * diff --git a/src/features/provider_account.feature b/src/features/provider_account.feature index e59dab9..92ec64f 100644 --- a/src/features/provider_account.feature +++ b/src/features/provider_account.feature @@ -23,8 +23,8 @@ Feature: Manage Provider Accounts And I should see "New Account" When I select "testprovider" from "provider_account_provider_id" And I fill in "provider_account[label]" with "testaccount" - And I fill in "provider_account[username]" with "mockuser" - And I fill in "provider_account[password]" with "mockpassword" + And I fill in "provider_account[credentials_hash][username]" with "mockuser" + And I fill in "provider_account[credentials_hash][password]" with "mockpassword" And I fill in "quota[maximum_running_instances]" with "13" And I press "Add" Then I should be on testaccount's provider account page @@ -39,8 +39,8 @@ Feature: Manage Provider Accounts And I am on the admin provider accounts page When I follow "New Account" Then I should be on the new admin provider account page - When I fill in "provider_account[username]" with "mockuser" - And I fill in "provider_account[password]" with "mockpassword" + When I fill in "provider_account[credentials_hash][username]" with "mockuser" + And I fill in "provider_account[credentials_hash][password]" with "mockpassword" And I press "Test Account" Then I should see "Test Connection Success" @@ -50,8 +50,8 @@ Feature: Manage Provider Accounts And I am on the admin provider accounts page When I follow "New Account" Then I should be on the new admin provider account page - When I fill in "provider_account[username]" with "mockuser" - And I fill in "provider_account[password]" with "wrong password" + When I fill in "provider_account[credentials_hash][username]" with "mockuser" + And I fill in "provider_account[credentials_hash][password]" with "wrong password" And I press "Test Account" Then I should see "Test Connection Failed" @@ -85,5 +85,5 @@ Feature: Manage Provider Accounts When I fill in "q" with "mock" And I press "Search" Then I should see the following: - | testaccount | mockuser | + | testaccount | mockuser | | otheraccount | mockuser | -- 1.7.4

Jan Provazník

2:01 p.m.

On 03/24/2011 04:38 PM, jzigmund(a)redhat.com wrote:

...

This patchset contains subtasks: #691 - Build credential model to support any provider, #693 - Update the generate_credentials method, #692 - Surface new creds model in 'add account form

Now patch works fine except one thing - I can still create accounts with same username/password. Tiny nit: this patchset doesn't include #693 - patch for 693 should generate credentials dynamically according to list of credential fields for specified provider type (i.e. there won't be fields x509* for mock accounts xml)

4843

days inactive

4843

days old

aeolus-devel@lists.fedorahosted.org

Manage subscription

3 comments

2 participants

tags (0)

participants (2)

Jan Provazník
jzigmund＠redhat.com

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

#690 Credentials model for provider account (rev.3)