On Fri, 2011-04-15 at 17:56 +0200, tsedovic@redhat.com wrote:
From: Tomas Sedovic tsedovic@redhat.com
This fixes Redmine #692
We're building the credentials XML that's passed to Image Factory dynamically using the credential_definitions table now.
The advantage is that as we add new providers, we won't have to update the build_credentials function. We populate the credential_definitions with the new entries and that will be it.
OK, so, parts of this look good like the updated build_credentials method, but I think there was a misunderstanding on some other parts (and this will affect the 2/2 patch, so I will not comment on that directly). I'll explain inline, so hopefully it makes more sense.
src/app/models/credential_definition.rb | 2 +- src/app/models/provider_account.rb | 35 +++++++++----------- ...31142700_change_credential_definitions_names.rb | 25 ++++++++++++++ src/db/seeds.rb | 14 ++++---- 4 files changed, 49 insertions(+), 27 deletions(-) create mode 100644 src/db/migrate/20110331142700_change_credential_definitions_names.rb
diff --git a/src/app/models/credential_definition.rb b/src/app/models/credential_definition.rb index b809425..556ebdf 100644 --- a/src/app/models/credential_definition.rb +++ b/src/app/models/credential_definition.rb @@ -20,5 +20,5 @@ class CredentialDefinition < ActiveRecord::Base validates_presence_of :label validates_presence_of :input_type validates_presence_of :provider_type_id
- CREDENTIAL_DEFINITIONS_ORDER = ["username", "password", "account_id", "x509private", "x509public"]
- CREDENTIAL_DEFINITIONS_ORDER = ['access_key', 'secret_access_key', 'account_number', 'key', 'certificate']
end diff --git a/src/app/models/provider_account.rb b/src/app/models/provider_account.rb index 9858219..c6f0079 100644 --- a/src/app/models/provider_account.rb +++ b/src/app/models/provider_account.rb @@ -170,25 +170,22 @@ class ProviderAccount < ActiveRecord::Base end
def build_credentials
- xml = Nokogiri::XML <<EOT
-<?xml version="1.0"?> -<provider_credentials>
<ec2_credentials>
- <account_number></account_number>
- <access_key></access_key>
- <secret_access_key></secret_access_key>
- <certificate></certificate>
- <key></key>
</ec2_credentials>
-</provider_credentials> -EOT
- node = xml.at_xpath('/provider_credentials/ec2_credentials')
- 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
doc = Nokogiri::XML('')
doc.root = Nokogiri::XML::Node.new('provider_credentials', doc)
root = doc.root.at_xpath('/provider_credentials')
credential_node_name = provider.provider_type.codename + '_credentials'
credential_node = Nokogiri::XML::Node.new(credential_node_name, doc)
root << credential_node
# The elements are sorted. That way we guarantee that the resulting XML is
# always the same which makes it easier to verify in tests.
credentials_hash.keys.sort.each do |k|
element = Nokogiri::XML::Node.new(k, doc)
element.content = credentials_hash[k]
credential_node << element
end
doc.to_xml end
def generate_auth_key
diff --git a/src/db/migrate/20110331142700_change_credential_definitions_names.rb b/src/db/migrate/20110331142700_change_credential_definitions_names.rb new file mode 100644 index 0000000..2536a54 --- /dev/null +++ b/src/db/migrate/20110331142700_change_credential_definitions_names.rb @@ -0,0 +1,25 @@ +class ChangeCredentialDefinitionsNames < ActiveRecord::Migration
- def self.up
- CredentialDefinition.all.each do |cred|
cred.name = name_mapping[cred.name]
cred.save!
- end
- end
- def self.down
- CredentialDefinition.all.each do |cred|
cred.name = name_mapping.invert[cred.name]
cred.save!
- end
- end
- def self.name_mapping
- {
"username" => "access_key",
"password" => "secret_access_key",
"account_id" => "account_number",
"x509private" => "key",
"x509public" => "certificate",
- }
- end
+end diff --git a/src/db/seeds.rb b/src/db/seeds.rb index 69be3f4..ab23138 100644 --- a/src/db/seeds.rb +++ b/src/db/seeds.rb @@ -126,16 +126,16 @@ end if CredentialDefinition.all.empty? ProviderType.all.each do |provider_type| unless provider_type.codename == 'ec2'
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 => 'access_key', :label => 'Username', :input_type => 'text', :provider_type_id => provider_type.id)
CredentialDefinition.create!(:name => 'secret_access_key', :label => 'Password', :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 => 'API Key', :input_type => 'text', :provider_type_id => ec2.id)
- CredentialDefinition.create!(:name => 'password', :label => 'Secret', :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)
- CredentialDefinition.create!(:name => 'access_key', :label => 'API Key', :input_type => 'text', :provider_type_id => ec2.id)
- CredentialDefinition.create!(:name => 'secret_access_key', :label => 'Secret', :input_type => 'password', :provider_type_id => ec2.id)
- CredentialDefinition.create!(:name => 'account_number', :label => 'AWS Account ID', :input_type => 'text', :provider_type_id => ec2.id)
- CredentialDefinition.create!(:name => 'key', :label => 'EC2 x509 private key', :input_type => 'file', :provider_type_id => ec2.id)
- CredentialDefinition.create!(:name => 'certificate', :label => 'EC2 x509 public key', :input_type => 'file', :provider_type_id => ec2.id)
The old values for 'name' here were almost correct in the old version (name and label are the reverse of what I designed, but otherwise had approximately correct values). I'll try to illustrate what is desired here with an example.
'username' is what dc api always uses, no matter what the provider is. EC2 may call it 'Access Key', but core still expects the access key value to be in the username field on the core http auth form. So, _if_ our form in the conductor view is using CredentialDefiniton.name as the id for the form fields, the id in this case should be 'username'. The CredentialDefiniton.label value should be used as the label _for that field in the form_, so in this instance 'Access Key'.
Now for the next level down. What we need to do when building the xml to send to the factory, is to transform (on the fly) 'Access Key' into 'access_key', which is (one of) the xml key(s) that the factory will parse out and use to build the actual image. The old xml in the build_credentials method is a good example of what is expected for ec2, so the final version of this should output exactly that when the method is called (which you do seem to have right now). Other providers may have some unknowns as far as what all these values are, but the values can easily be added as we start using/building for another provider. The main thing here is the convention factory is expecting is a tag with whatever the provider calls this api key _transformed_ to lowercase, with each word separated by an underscore. I am starting to wonder if perhaps it would be clearer if the definition fields were call core_name and provider_name, but maybe that would be confusing for the credentials core does not give us.
I hope this makes more sense, and should actually not be a lot of work to update your implementation to work this way, as you already have a large chunk correct of what is needed.
-j
end