Merge "Support for services different only by type."

lib/puppet/provider/keystone_service/openstack.rb

@@ -9,23 +9,24 @@ Puppet::Type.type(:keystone_service).provide(
 
   @credentials = Puppet::Provider::Openstack::CredentialsV3.new
 
-  def initialize(value={})
+  include PuppetX::Keystone::CompositeNamevar::Helpers
+
+  def initialize(value = {})
     super(value)
     @property_flush = {}
   end
 
   def create
-    if resource[:type]
+    properties = [resource[:type]]
-      properties = [resource[:type]]
+    properties << '--name' << resource[:name]
-      properties << '--name' << resource[:name]
+    if resource[:description]
-      if resource[:description]
+      properties << '--description' << resource[:description]
-        properties << '--description' << resource[:description]
-      end
-      self.class.request('service', 'create', properties)
-      @property_hash[:ensure] = :present
-    else
-      raise(Puppet::Error, 'The type is mandatory for creating a keystone service')
     end
+    created = self.class.request('service', 'create', properties)
+    @property_hash[:ensure] = :present
+    @property_hash[:type] = resource[:type]
+    @property_hash[:id] = created[:id]
+    @property_hash[:description] = resource[:description]
   end
 
   def destroy
@@ -37,31 +38,21 @@ Puppet::Type.type(:keystone_service).provide(
     @property_hash[:ensure] == :present
   end
 
-  def description
+  mk_resource_methods
-    @property_hash[:description]
-  end
 
   def description=(value)
     @property_flush[:description] = value
   end
 
-  def type
-    @property_hash[:type]
-  end
-
   def type=(value)
     @property_flush[:type] = value
   end
 
-  def id
-    @property_hash[:id]
-  end
-
   def self.instances
     list = request('service', 'list', '--long')
     list.collect do |service|
       new(
-        :name        => service[:name],
+        :name        => resource_to_name(service[:type], service[:name], false),
         :ensure      => :present,
         :type        => service[:type],
         :description => service[:description],
@@ -71,11 +62,10 @@ Puppet::Type.type(:keystone_service).provide(
   end
 
   def self.prefetch(resources)
-    services = instances
+    prefetch_composite(resources) do |sorted_namevars|
-    resources.keys.each do |name|
+      name = sorted_namevars[0]
-       if provider = services.find{ |service| service.name == name }
+      type = sorted_namevars[1]
-        resources[name].provider = provider
+      resource_to_name(type, name, false)
-      end
     end
   end
 

lib/puppet/type/keystone_service.rb

@@ -1,6 +1,8 @@
 # LP#1408531
 File.expand_path('../..', File.dirname(__FILE__)).tap { |dir| $LOAD_PATH.unshift(dir) unless $LOAD_PATH.include?(dir) }
 File.expand_path('../../../../openstacklib/lib', File.dirname(__FILE__)).tap { |dir| $LOAD_PATH.unshift(dir) unless $LOAD_PATH.include?(dir) }
+require 'puppet_x/keystone/composite_namevar'
+require 'puppet_x/keystone/type'
 
 Puppet::Type.newtype(:keystone_service) do
 
@@ -14,16 +16,13 @@ Puppet::Type.newtype(:keystone_service) do
   end
 
   newproperty(:id) do
-    validate do |v|
+    include PuppetX::Keystone::Type::ReadOnly
-      raise(Puppet::Error, 'This is a read only property')
-    end
   end
 
-  newproperty(:type) do
+  newparam(:type) do
+    isnamevar
     desc 'The type of service'
-    validate do |value|
+    include PuppetX::Keystone::Type::Required
-      fail('The service type is required.') unless value
-    end
   end
 
   newproperty(:description) do
@@ -38,4 +37,8 @@ Puppet::Type.newtype(:keystone_service) do
   autorequire(:anchor) do
     ['keystone_started']
   end
+
+  def self.title_patterns
+    PuppetX::Keystone::CompositeNamevar.basic_split_title_patterns(:name, :type)
+  end
 end

lib/puppet_x/keystone/type.rb

@@ -1 +1,3 @@
 require 'puppet_x/keystone/type/default_domain'
+require 'puppet_x/keystone/type/required'
+require 'puppet_x/keystone/type/read_only'

lib/puppet_x/keystone/type/read_only.rb

@@ -0,0 +1,15 @@
+module PuppetX
+  module Keystone
+    module Type
+      module ReadOnly
+        def self.included(klass)
+          klass.class_eval do
+            validate do |_|
+              fail(ArgumentError, 'Read-only property.')
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/puppet_x/keystone/type/required.rb

@@ -0,0 +1,18 @@
+module PuppetX
+  module Keystone
+    module Type
+      module Required
+        def self.included(klass)
+          klass.class_eval do
+            defaultto do
+              fail(Puppet::ResourceError,
+                   "Parameter #{name} failed on " \
+                     "#{resource.class.to_s.split('::')[-1]}[#{resource.name}]: " \
+                     'Required parameter.')
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/acceptance/basic_keystone_spec.rb

@@ -272,4 +272,16 @@ describe 'basic keystone server with resources' do
       end
     end
   end
+  describe 'composite namevar for keystone_service' do
+    let(:pp) do
+      <<-EOM
+      keystone_service { 'service_1::type_1': ensure => present }
+      keystone_service { 'service_1': type => 'type_2', ensure => present }
+      EOM
+    end
+    it 'should be possible to create two services different only by their type' do
+      apply_manifest(pp, :catch_failures => true)
+      apply_manifest(pp, :catch_changes => true)
+    end
+  end
 end

spec/acceptance/keystone_wsgi_apache_spec.rb

@@ -239,6 +239,45 @@ describe 'keystone server running with Apache/WSGI with resources' do
       include_examples 'keystone user/tenant/service/role/endpoint resources using v3 API',
                        '--os-username beaker-civ3 --os-password secret --os-project-name servicesv3 --os-user-domain-name service_domain --os-project-domain-name service_domain'
     end
-
+  end
+  describe 'composite namevar quick test' do
+    context 'similar resources different naming' do
+      let(:pp) do
+        <<-EOM
+        keystone_tenant { 'openstackv3':
+          ensure      => present,
+          enabled     => true,
+          description => 'admin tenant',
+          domain      => 'admin_domain'
+        }
+        keystone_user { 'adminv3::useless_when_the_domain_is_set':
+          ensure      => present,
+          enabled     => true,
+          email       => 'test@example.tld',
+          password    => 'a_big_secret',
+          domain      => 'admin_domain'
+        }
+        keystone_user_role { 'adminv3::admin_domain@openstackv3::admin_domain':
+          ensure         => present,
+          roles          => ['admin'],
+        }
+        EOM
+      end
+      it 'should not do any modification' do
+        apply_manifest(pp, :catch_changes => true)
+      end
+    end
+  end
+  describe 'composite namevar for keystone_service' do
+    let(:pp) do
+      <<-EOM
+      keystone_service { 'service_1::type_1': ensure => present }
+      keystone_service { 'service_1': type => 'type_2', ensure => present }
+      EOM
+    end
+    it 'should be possible to create two services different only by their type' do
+      apply_manifest(pp, :catch_failures => true)
+      apply_manifest(pp, :catch_changes => true)
+    end
   end
 end

spec/shared_examples.rb

@@ -8,10 +8,7 @@ shared_examples_for 'parse title correctly' do |result|
   let(:title) do |example|
     example.metadata[:example_group][:description]
   end
-  let(:current_class) do |example|
+  let(:resource) { described_class.new(:title => title) }
-    example.metadata[:described_class]
-  end
-  let(:resource) { current_class.new(:title => title) }
   it 'should parse this title correctly' do
     times = result.delete(:calling_default) || 0
     Puppet::Provider::Keystone.expects(:default_domain).times(times).returns('Default')
@@ -23,12 +20,41 @@ shared_examples_for 'croak on the title' do
   let(:title) do |example|
     example.metadata[:example_group][:description]
   end
-  let(:current_class) do |example|
+  let(:resource) { described_class.new(:title => title) }
-    example.metadata[:described_class]
-  end
-  let(:user) { current_class.new(:title => title) }
   it 'croak on the title' do
-    expect { user }.to raise_error(Puppet::Error, /No set of title patterns matched the title/)
+    expect { resource }.to raise_error(Puppet::Error, /No set of title patterns matched the title/)
+  end
+end
+
+shared_examples_for 'croak on the required parameter' do |attr|
+  let(:title) do |example|
+    example.metadata[:example_group][:description]
+  end
+  prefix = attr.is_a?(String) ? attr : ''
+
+  let(:resource) { described_class.new(:title => title) }
+  it 'croak on the missing required parameter' do
+    expect { resource }
+      .to raise_error(Puppet::ResourceError, "#{prefix} Required parameter.")
+  end
+end
+
+shared_examples_for 'croak on read-only parameter' do |resource|
+  prefix = resource.delete(:_prefix)
+  it 'should raise an error' do
+    expect { described_class.new(resource) }
+      .to raise_error(Puppet::ResourceError, "#{prefix} Read-only property.")
+  end
+end
+
+shared_examples_for 'succeed with the required parameters' do |extra_params|
+  let(:title) do |example|
+    example.metadata[:example_group][:description]
+  end
+  extra_params_to_merge = extra_params || {}
+  let(:resource) { described_class.new({ :title => title }.merge(extra_params_to_merge)) }
+  it 'has all required parameters' do
+    expect { resource }.not_to raise_error
   end
 end
 
@@ -51,9 +77,6 @@ end
 
 # Let resources to [<existing>, <non_existing>]
 shared_examples_for 'prefetch the resources' do
-  let(:current_class) do |example|
-    example.metadata[:described_class]
-  end
   it 'should correctly prefetch the existing resource' do
     existing     = resources[0]
     non_existing = resources[1]
@@ -68,8 +91,8 @@ shared_examples_for 'prefetch the resources' do
     resource.expects(:values).returns(m_value)
     m_value.expects(:first).returns(m_first)
     m_first.expects(:catalog).returns(catalog)
-    m_first.expects(:class).returns(current_class.resource_type)
+    m_first.expects(:class).returns(described_class.resource_type)
-    current_class.prefetch(resource)
+    described_class.prefetch(resource)
 
     # found and not found
     expect(existing.provider.ensure).to eq(:present)
@@ -81,7 +104,8 @@ end
 # - the first hash are the expected result
 # - second are parameters to test default domain, required but can be empty
 # - the rest are the combination of attributes you want to test
-# see examples in user/user_role/tenant
+# The provider must be build from ressource_attrs
+# see examples in keystone_{user/user_role/tenant/service}
 shared_examples_for 'create the correct resource' do |attributes|
   expected_results = attributes.shift['expected_results']
   default_domain   = attributes.shift

spec/unit/provider/keystone_service/openstack_spec.rb

@@ -17,19 +17,19 @@ describe provider_class do
     set_env
   end
 
-  describe 'when managing a service' do
+  describe 'when managing service' do
 
-    let(:service_attrs) do
+    let(:resource_attrs) do
       {
-        :name         => 'foo',
+        :name         => 'service_one',
-        :description  => 'foo',
+        :description  => 'Service One',
         :ensure       => 'present',
-        :type         => 'foo',
+        :type         => 'type_one'
       }
     end
 
     let(:resource) do
-      Puppet::Type::Keystone_service.new(service_attrs)
+      Puppet::Type::Keystone_service.new(resource_attrs)
     end
 
     let(:provider) do
@@ -37,63 +37,100 @@ describe provider_class do
     end
 
     describe '#create' do
-      it 'creates a service' do
+      before(:each) do
-        provider.class.stubs(:openstack)
+        provider_class.expects(:openstack)
-          .with('service', 'list', '--quiet', '--format', 'csv', '--long')
+          .with('service', 'create', '--format', 'shell',
-          .returns('"ID","Name","Type","Description"
+                ['type_one', '--name', 'service_one', '--description', 'Service One'])
-"1cb05cfed7c24279be884ba4f6520262","foo","foo","foo"
+          .returns('description="Service One"
-'
-                  )
-        provider.class.stubs(:openstack)
-          .with('service', 'create', '--format', 'shell', ['foo', '--name', 'foo', '--description', 'foo'])
-          .returns('description="foo"
 enabled="True"
 id="8f0dd4c0abc44240998fbb3f5089ecbf"
-name="foo"
+name="service_one"
-type="foo"
+type="type_one"
-'
+')
-                  )
+      end
-        provider.create
+      include_examples 'create the correct resource', [
-        expect(provider.exists?).to be_truthy
+        {
+          'expected_results' => {
+            :type        => 'type_one',
+            :id          => '8f0dd4c0abc44240998fbb3f5089ecbf',
+            :name        => 'service_one',
+            :description => 'Service One'
+          }
+        },
+        {},
+        {
+          'type in title' => {
+            :title       => 'service_one::type_one',
+            :description => 'Service One'
+          }
+        },
+        {
+          'type in parameter' => {
+            :title       => 'service_one',
+            :type        => 'type_one',
+            :description => 'Service One'
+          }
+        }
+      ]
+
+    end
+    describe '#destroy' do
+      it 'destroys a service' do
+        provider_class.expects(:openstack)
+          .with('service', 'delete', [])
+        provider.destroy
+        expect(provider.exists?).to be_falsey
       end
 
-      describe '#destroy' do
+      context 'when service does not exist' do
-        it 'destroys a service' do
+        subject(:response) do
-          provider.class.stubs(:openstack)
+          provider.exists?
-            .with('service', 'list', '--quiet', '--format', 'csv', '--long')
-            .returns('"ID","Name","Type","Description"
-"1cb05cfed7c24279be884ba4f6520262","foo","foo","foo"
-'
-                    )
-          provider.class.stubs(:openstack)
-            .with('service', 'delete', [])
-          provider.destroy
-          expect(provider.exists?).to be_falsey
-        end
-
-        context 'when service does not exist' do
-          subject(:response) do
-            provider.class.stubs(:openstack)
-              .with('service', 'list', '--quiet', '--format', 'csv', '--long')
-              .returns('"ID","Name","Type","Description"')
-            response = provider.exists?
-          end
-          it { is_expected.to be_falsey }
         end
+        it { is_expected.to be_falsey }
       end
+    end
 
-      describe '#instances' do
+    describe '#instances' do
-        it 'finds every service' do
+      it 'finds every service' do
-          provider.class.stubs(:openstack)
+        provider_class.expects(:openstack)
-            .with('service', 'list', '--quiet', '--format', 'csv', '--long')
+          .with('service', 'list', '--quiet', '--format', 'csv', '--long')
-            .returns('"ID","Name","Type","Description"
+          .returns('"ID","Name","Type","Description"
-"8f0dd4c0abc44240998fbb3f5089ecbf","foo","foo","foo"
+"8f0dd4c0abc44240998fbb3f5089ecbf","service_one","type_one","Service One"
-'
+')
-                    )
+        instances = provider_class.instances
-          instances = Puppet::Type::Keystone_service::ProviderOpenstack.instances
+        expect(instances.count).to eq(1)
-          expect(instances.count).to eq(1)
-        end
       end
     end
   end
+
+  context '#prefetch' do
+    before(:each) do
+      # This call done by self.instance in prefetch in what make the
+      # resource exists.
+      provider_class.expects(:openstack)
+        .with('service', 'list', '--quiet', '--format', 'csv', '--long')
+        .returns('"ID","Name","Type","Description"
+"8f0dd4c0abc44240998fbb3f5089ecbf","service_1","type_1",""
+')
+    end
+    let(:service_1) do
+      Puppet::Type::Keystone_service.new(:title => 'service_1::type_1')
+    end
+    let(:service_2) do
+      Puppet::Type::Keystone_service.new(:title => 'service_1', :type => 'type_2')
+    end
+    let(:resources) { [service_1, service_2] }
+    include_examples 'prefetch the resources'
+  end
+
+  context 'duplicate detection' do
+    let(:service_1) do
+      Puppet::Type::Keystone_service.new(:title => 'service_1::type_1')
+    end
+    let(:service_2) do
+      Puppet::Type::Keystone_service.new(:title => 'service_1', :type => 'type_1')
+    end
+    let(:resources) { [service_1, service_2] }
+    include_examples 'detect duplicate resource'
+  end
 end

spec/unit/type/keystone_service_spec.rb

@@ -0,0 +1,32 @@
+require 'spec_helper'
+require 'puppet'
+require 'puppet/type/keystone_service'
+
+describe Puppet::Type.type(:keystone_service) do
+
+  let(:project) do
+    Puppet::Type.type(:keystone_service).new(
+      :id   => 'blah',
+      :name => 'foo',
+      :type => 'foo-type'
+    )
+  end
+
+  include_examples 'croak on read-only parameter',
+    :title => 'service::type', :id => '12345',
+    :_prefix => 'Parameter id failed on Keystone_service[service::type]:'
+
+  describe 'service::type' do
+    include_examples 'parse title correctly', :name => 'service', :type => 'type'
+  end
+
+  describe 'new_service_without_type' do
+    include_examples 'croak on the required parameter',
+      'Parameter type failed on Keystone_service[new_service_without_type]:'
+  end
+
+  describe 'new_service_with_type_as_parameter' do
+    include_examples 'succeed with the required parameters', :type => 'type'
+  end
+
+end