Allow create_user to be idempotent

Was updating the password all the time which would invalidate any of
the users existing tokens and might cause temp auth issues.

This change adds a token-get check to allow the password update to be
skipped if it's already current.

Broke down the provider CLI Environment into three categories (from two).
* Boot is using the bootstrap token
* Admin is using admin endpoint and admin creds, only used by EC2.
* User is using api endpoint and user creds, used for token-get.

The current provider design has the basic input parms (user_name,
user_password, tenant_name) and the ones needed only for implementation
all required.   I think the implementation ones should all be optional
as the provider can easily get them for itself.  On that thought, with
this change I started with allowing identity_endpoint to be optional.
It is used only for the create_user and create_ec2_credentials ascions.
I believe this is the right direction for this provider.

Having said all that, I also know that we have begun work on the client
cookbook to have providers for many OpenStack resouces like keystone
items. Since that is not far enough along yet, I decided to make the
changes in the currently used identity provider.

Added missing EC2 tests.
Added logging of the CLI Environment.

Change-Id: Iec2eb748e20389ceb70c25d8bfcfd078e6de67ad
Closes-Bug: #1383821

CHANGELOG.md

@@ -13,6 +13,7 @@ This file is used to list changes made in each version of cookbook-openstack-ide
 * Add test to verify each endpoint can be configured seperatly
 * Update endpoint when endpoint for one service type exists
 * Add attributes for pipeline of API
+* Add create user idempotent password check
 
 ## 10.0.0
 * Upgrading to Juno

providers/register.rb

@@ -27,7 +27,7 @@ include ::Openstack
 
 private
 
-def generate_creds(resource)
+def generate_boot_creds(resource)
   {
     'OS_SERVICE_ENDPOINT' => resource.auth_uri,
     'OS_SERVICE_TOKEN' => resource.bootstrap_token
@@ -36,25 +36,54 @@ end
 
 private
 
-def generate_ec2_creds(resource)
+def generate_admin_creds(resource)
+  identity_endpoint = resource.identity_endpoint
+  identity_endpoint = endpoint('identity-admin').to_s unless identity_endpoint
   {
       'OS_USERNAME' => resource.admin_user,
       'OS_PASSWORD' => resource.admin_pass,
       'OS_TENANT_NAME' => resource.admin_tenant_name,
-      'OS_AUTH_URL' => resource.identity_endpoint
+      'OS_AUTH_URL' => identity_endpoint
   }
 end
 
 private
 
-def identity_command(resource, cmd, args = {})
+def generate_user_creds(resource)
+  identity_endpoint = resource.identity_endpoint
+  identity_endpoint = endpoint('identity-api').to_s unless identity_endpoint
+  {
+      'OS_USERNAME' => resource.user_name,
+      'OS_PASSWORD' => resource.user_pass,
+      'OS_TENANT_NAME' => resource.tenant_name,
+      'OS_AUTH_URL' => identity_endpoint
+  }
+end
+
+private
+
+def get_env(resource, env = 'boot')
+  case env
+  when 'boot'
+    generate_boot_creds(resource)
+  when 'user'
+    generate_user_creds(resource)
+  when 'admin'
+    generate_admin_creds(resource)
+  end
+end
+
+private
+
+def identity_command(resource, cmd, args = {}, env = 'boot')
   keystonecmd = ['keystone'] << '--insecure' << cmd
   args.each do |key, val|
     keystonecmd << "--#{key}" unless key.empty?
     keystonecmd << val.to_s
   end
-  Chef::Log.debug("Running identity command: #{keystonecmd}")
-  rc = shell_out(keystonecmd, env: (cmd.include? 'ec2') ? generate_ec2_creds(resource) : generate_creds(resource))
+  cmd_env = get_env(resource, env)
+  Chef::Log.debug("Running identity command: #{keystonecmd} env: " + cmd_env.to_s)
+  rc = shell_out(keystonecmd, env: cmd_env)
   fail "#{rc.stderr} (#{rc.exitstatus})" if rc.exitstatus != 0
   rc.stdout
 end
@@ -213,12 +242,17 @@ action :create_user do
 
     if user_found
       Chef::Log.info("User '#{new_resource.user_name}' already exists for tenant '#{new_resource.tenant_name}'")
-      # Make sure password is always up to date. Leaving updated_by_last_action to false as there's no way to tell
-      # if the password was actually updated or was the same as before.
-      Chef::Log.info("Sync password for user '#{new_resource.user_name}'")
-      identity_command(new_resource, 'user-password-update',
-                       'pass' => new_resource.user_pass,
-                       '' => new_resource.user_name)
+      begin
+        # Check if password is already updated by getting a token
+        identity_command(new_resource, 'token-get', {}, 'user')
+      rescue StandardError => e
+        Chef::Log.debug('Get token error:' + e.message)
+        Chef::Log.info("Sync password for user '#{new_resource.user_name}'")
+        identity_command(new_resource, 'user-password-update',
+                         'pass' => new_resource.user_pass,
+                         '' => new_resource.user_name)
+        new_resource.updated_by_last_action(true)
+      end
       next
     end
 
@@ -282,8 +316,9 @@ action :create_ec2_credentials do
       Chef::Log.info("EC2 credentials already exist for '#{new_resource.user_name}' in tenant '#{new_resource.tenant_name}'")
     else
       output = identity_command(new_resource, 'ec2-credentials-create',
-                                'user-id' => user_uuid,
-                                'tenant-id' => tenant_uuid)
+                                { 'user-id' => user_uuid,
+                                  'tenant-id' => tenant_uuid },
+                                'admin')
       Chef::Log.info("Created EC2 Credentials for User '#{new_resource.user_name}' in Tenant '#{new_resource.tenant_name}'")
       data = prettytable_to_array(output)
 

recipes/registration.rb

@@ -140,7 +140,6 @@ node['openstack']['identity']['users'].each do |username, user_info|
     admin_tenant_name admin_tenant_name
     admin_user admin_user
     admin_pass admin_pass
-    identity_endpoint identity_endpoint.to_s
 
     action :create_ec2_credentials
   end

resources/register.rb

@@ -65,4 +65,7 @@ attribute :role_name, kind_of: String
 attribute :admin_tenant_name, kind_of: String
 attribute :admin_user, kind_of: String
 attribute :admin_pass, kind_of: String
+
+# Used by create_ec2_credentials and create_user
+# If not specified, default endpoint will be used.
 attribute :identity_endpoint, kind_of: String

spec/register_spec.rb

@@ -453,7 +453,7 @@ describe 'openstack-identity::default' do
         end
       end
 
-      context 'when user already exist' do
+      context 'when user already exist with same password' do
         before do
           allow(provider).to receive(:identity_uuid)
             .with(resource, 'tenant', 'name', 'tenant1')
@@ -467,9 +467,7 @@ describe 'openstack-identity::default' do
             .with(resource, 'user', 'name', 'user1')
             .and_return('HGFEDCBA0987654321')
           allow(provider).to receive(:identity_command)
-            .with(resource, 'user-password-update',
-                  'pass' => 'password',
-                  '' => 'user1')
+            .with(resource, 'token-get', {}, 'user')
         end
 
         it 'should not create a user' do
@@ -478,6 +476,34 @@ describe 'openstack-identity::default' do
         end
       end
 
+      context 'when user already exist and changed password' do
+        before do
+          allow(provider).to receive(:identity_uuid)
+            .with(resource, 'tenant', 'name', 'tenant1')
+            .and_return('1234567890ABCDEFGH')
+          allow(provider).to receive(:identity_command)
+            .with(resource, 'user-list',
+                  'tenant-id' => '1234567890ABCDEFGH')
+          allow(provider).to receive(:prettytable_to_array)
+            .and_return([{ 'name' => 'user1' }])
+          allow(provider).to receive(:identity_uuid)
+            .with(resource, 'user', 'name', 'user1')
+            .and_return('HGFEDCBA0987654321')
+          allow(provider).to receive(:identity_command)
+            .with(resource, 'token-get', {}, 'user')
+            .and_raise('Error!')
+          allow(provider).to receive(:identity_command)
+            .with(resource, 'user-password-update',
+                  'pass' => 'password',
+                  '' => 'user1')
+        end
+
+        it 'should update user password' do
+          provider.run_action(:create_user)
+          expect(resource).to be_updated
+        end
+      end
+
       describe '#identity_command' do
         it 'should handle false values and long descriptions' do
           allow(provider).to receive(:shell_out)
@@ -621,8 +647,9 @@ describe 'openstack-identity::default' do
                   { 'user-id' => 'HGFEDCBA0987654321' }, 'access')
           allow(provider).to receive(:identity_command)
             .with(resource, 'ec2-credentials-create',
-                  'user-id' => 'HGFEDCBA0987654321',
-                  'tenant-id' => '1234567890ABCDEFGH')
+                  { 'user-id' => 'HGFEDCBA0987654321',
+                    'tenant-id' => '1234567890ABCDEFGH' },
+                  'admin')
           allow(provider).to receive(:prettytable_to_array)
             .and_return([{ 'access' => 'access', 'secret' => 'secret' }])
         end

spec/registration_spec.rb

@@ -124,6 +124,19 @@ describe 'openstack-identity::registration' do
               end
             end
 
+            it 'registers the admin user for ec2' do
+              expect(chef_run).to create_ec2_credentials_openstack_identity_register(
+                "Create EC2 credentials for 'admin' user"
+              ).with(
+                auth_uri: 'http://127.0.0.1:35357/v2.0',
+                bootstrap_token: 'bootstrap-token',
+                user_name: user,
+                tenant_name: tenant,
+                admin_tenant_name: 'admin',
+                admin_user: 'admin',
+                admin_pass: ''
+              )
+            end
           end
         end
       end
@@ -154,6 +167,20 @@ describe 'openstack-identity::registration' do
             tenant_name: 'role_tenant1'
           )
         end
+
+        it 'registers the user1 user for ec2' do
+          expect(chef_run).to create_ec2_credentials_openstack_identity_register(
+            "Create EC2 credentials for 'user1' user"
+          ).with(
+            auth_uri: 'http://127.0.0.1:35357/v2.0',
+            bootstrap_token: 'bootstrap-token',
+            user_name: 'user1',
+            tenant_name: 'default_tenant1',
+            admin_tenant_name: 'admin',
+            admin_user: 'admin',
+            admin_pass: ''
+          )
+        end
       end
     end