diff --git a/lib/puppet/provider/liu_pulpcore_group/liu_pulpcore_group.rb b/lib/puppet/provider/liu_pulpcore_group/liu_pulpcore_group.rb index 4218a352d5acf3ddc34c15773635f929646a3bda..8b67d12fdd6b56b957675bf773e567d5e719c66d 100644 --- a/lib/puppet/provider/liu_pulpcore_group/liu_pulpcore_group.rb +++ b/lib/puppet/provider/liu_pulpcore_group/liu_pulpcore_group.rb @@ -19,12 +19,19 @@ class Puppet::Provider::LiuPulpcoreGroup::LiuPulpcoreGroup < Puppet::ResourceApi result[:pulp_href] = group['pulp_href'] result[:raw_members] = paginated { request("#{group['pulp_href']}/users/") }['results'] - result[:members] = result[:raw_members].map { |member| member['username'] } + result[:members] = result[:raw_members].map { |member| member['username'] }.sort result end end + def canonicalize(_context, resources) + resources.each do |group| + group[:members]&.sort! + end + resources + end + def create(_context, name, should) data = execute('create', name, should) cache << data.transform_keys(&:to_sym).slice(:pulp_href, *attribute_keys).merge(name: name) diff --git a/lib/puppet/provider/liu_pulpcore_group_role/liu_pulpcore_group_role.rb b/lib/puppet/provider/liu_pulpcore_group_role/liu_pulpcore_group_role.rb index ce0a8f9a57c05411d801b1ab62c2737eb9578f6c..08c526db46ba0c5a59bcee12a17d7c29ac467f15 100644 --- a/lib/puppet/provider/liu_pulpcore_group_role/liu_pulpcore_group_role.rb +++ b/lib/puppet/provider/liu_pulpcore_group_role/liu_pulpcore_group_role.rb @@ -30,6 +30,9 @@ class Puppet::Provider::LiuPulpcoreGroupRole::LiuPulpcoreGroupRole < Puppet::Res result[:base_href] = group['pulp_href'] result[:raw_roles] = paginated(per_page: 1000) { request("#{group['pulp_href']}/roles/", query: { exclude_fields: 'permissions,description', role: namevar[:role] }) }['results'] result.merge! parse_roles(result[:raw_roles], workaround: true).transform_keys(&:to_sym) + + result[:object].sort_by!(&:to_s) + result[:domain].sort_by!(&:to_s) end result @@ -37,6 +40,14 @@ class Puppet::Provider::LiuPulpcoreGroupRole::LiuPulpcoreGroupRole < Puppet::Res end end + def canonicalize(_context, resources) + resources.each do |group_role| + group_role[:object]&.sort_by!(&:to_s) + group_role[:domain]&.sort_by!(&:to_s) + end + resources + end + def create(_context, name, should) execute('create', name, should) end diff --git a/lib/puppet/provider/liu_pulpcore_role/liu_pulpcore_role.rb b/lib/puppet/provider/liu_pulpcore_role/liu_pulpcore_role.rb index f17972bb8e70fac89b436e610ba567b50269421f..d11093c1c71048f27a7f5a14e401316c6fcf4931 100644 --- a/lib/puppet/provider/liu_pulpcore_role/liu_pulpcore_role.rb +++ b/lib/puppet/provider/liu_pulpcore_role/liu_pulpcore_role.rb @@ -18,10 +18,18 @@ class Puppet::Provider::LiuPulpcoreRole::LiuPulpcoreRole < Puppet::ResourceApi:: result[:name] = role['name'] result[:pulp_href] = role['pulp_href'] result[:locked] = role['locked'] + result[:permissions].sort! result end end + def canonicalize(_context, resources) + resources.each do |role| + role[:permissions]&.sort_by!(&:to_s) + end + resources + end + def create(_context, name, should) data = execute('create', name, should) cache << data.transform_keys(&:to_sym).slice(:pulp_href, *attribute_keys).merge(name: name) diff --git a/lib/puppet/provider/liu_pulpcore_user_role/liu_pulpcore_user_role.rb b/lib/puppet/provider/liu_pulpcore_user_role/liu_pulpcore_user_role.rb index 4b18d51e3784d693efeee37abe5e69b3a086b796..5b4244f2c5e416ad7ba451e7071a6abc96c9cbe6 100644 --- a/lib/puppet/provider/liu_pulpcore_user_role/liu_pulpcore_user_role.rb +++ b/lib/puppet/provider/liu_pulpcore_user_role/liu_pulpcore_user_role.rb @@ -30,6 +30,9 @@ class Puppet::Provider::LiuPulpcoreUserRole::LiuPulpcoreUserRole < Puppet::Resou result[:base_href] = user['pulp_href'] result[:raw_roles] = paginated(per_page: 1000) { request("#{user['pulp_href']}/roles/", query: { exclude_fields: 'permissions,description', role: namevar[:role] }) }['results'] result.merge! parse_roles(result[:raw_roles], workaround: true).transform_keys(&:to_sym) + + result[:object].sort_by!(&:to_s) + result[:domain].sort_by!(&:to_s) end result @@ -37,6 +40,14 @@ class Puppet::Provider::LiuPulpcoreUserRole::LiuPulpcoreUserRole < Puppet::Resou end end + def canonicalize(_context, resources) + resources.each do |user_role| + user_role[:object]&.sort_by!(&:to_s) + user_role[:domain]&.sort_by!(&:to_s) + end + resources + end + def create(_context, name, should) execute('create', name, should) end diff --git a/lib/puppet/type/liu_pulpcore_group.rb b/lib/puppet/type/liu_pulpcore_group.rb index 588ced0cf04599cdc2f5d8c30733fc2803ab1484..1aa792f88677061459ef53440d7f5e2f16a51396 100644 --- a/lib/puppet/type/liu_pulpcore_group.rb +++ b/lib/puppet/type/liu_pulpcore_group.rb @@ -12,7 +12,7 @@ Puppet::ResourceApi.register_type( members => [ 'dev-user' ], } EOS - features: [], + features: ['canonicalize'], attributes: { ensure: { type: 'Enum[present, absent]', diff --git a/lib/puppet/type/liu_pulpcore_group_role.rb b/lib/puppet/type/liu_pulpcore_group_role.rb index 13b582d0e03026af268b691fa5fef405f0bd18c0..df9cc8cfef6b0cf4920cf873d1dcd5b917343a9e 100644 --- a/lib/puppet/type/liu_pulpcore_group_role.rb +++ b/lib/puppet/type/liu_pulpcore_group_role.rb @@ -22,7 +22,7 @@ Puppet::ResourceApi.register_type( object => [Liu_pulpcore_rpm_repository['testing']], } EOS - features: ['simple_get_filter'], + features: ['simple_get_filter', 'canonicalize'], title_patterns: [ { pattern: %r{\A(?<group>[^/]+)/(?<role>.+)\Z}, diff --git a/lib/puppet/type/liu_pulpcore_role.rb b/lib/puppet/type/liu_pulpcore_role.rb index 2a5da6aebe79d81e87622c17583255e8311fcaa7..4eba1151a146fbb46380bfbc8f2599edc0035c12 100644 --- a/lib/puppet/type/liu_pulpcore_role.rb +++ b/lib/puppet/type/liu_pulpcore_role.rb @@ -7,7 +7,7 @@ Puppet::ResourceApi.register_type( docs: <<-EOS, @summary Pulp RBAC role EOS - features: [], + features: ['canonicalize'], attributes: { ensure: { type: 'Enum[present, absent]', diff --git a/lib/puppet/type/liu_pulpcore_user_role.rb b/lib/puppet/type/liu_pulpcore_user_role.rb index ba437dbc1e5107a9a35ec269d5b8981f44db89ad..f5bfa0fc8786ef5b25e64994d9adb7259c688891 100644 --- a/lib/puppet/type/liu_pulpcore_user_role.rb +++ b/lib/puppet/type/liu_pulpcore_user_role.rb @@ -22,7 +22,7 @@ Puppet::ResourceApi.register_type( object => [Liu_pulpcore_rpm_repository['testing']], } EOS - features: ['simple_get_filter'], + features: ['simple_get_filter', 'canonicalize'], title_patterns: [ { pattern: %r{\A(?<user>[^/]+)/(?<role>.+)\Z}, diff --git a/spec/fixtures/pulp_data/get_group_users.json b/spec/fixtures/pulp_data/get_group_users.json index d53b47727ef3ce642212d64ae2f9b648b143b904..37b3eb67131a326d03dffabea80e13e48a00e372 100644 --- a/spec/fixtures/pulp_data/get_group_users.json +++ b/spec/fixtures/pulp_data/get_group_users.json @@ -4,7 +4,11 @@ "previous": null, "results": [ { - "username": "example-user", + "username": "example-user2", + "pulp_href": "/pulp/api/v3/users/6/" + }, + { + "username": "example-user1", "pulp_href": "/pulp/api/v3/users/5/" } ] diff --git a/spec/integration/liu_pulpcore/catalog_spec.rb b/spec/integration/liu_pulpcore/catalog_spec.rb index d9bd581015de6a7772efdd70b51e608d4003df0e..be69025c13666bdade287019cb99078f1f000750 100644 --- a/spec/integration/liu_pulpcore/catalog_spec.rb +++ b/spec/integration/liu_pulpcore/catalog_spec.rb @@ -335,7 +335,9 @@ RSpec.describe 'LiU_pulpcore integration test' do body: '{}', ) - catalog.apply + report = catalog.apply.report + report.finalize_report + expect(report.status).to eq('changed') # Ensure all resources have been created assert_requested rpm_remote diff --git a/spec/integration/liu_pulpcore/member_ordering_spec.rb b/spec/integration/liu_pulpcore/member_ordering_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..487a062dc97af3566138f9ff01c6dad07210b647 --- /dev/null +++ b/spec/integration/liu_pulpcore/member_ordering_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'LiU_pulpcore member ordering integration test' do + let(:group_members) { ['example-user1', 'example-user2'] } + let(:group) { Puppet::Type.type(:liu_pulpcore_group).new(name: 'example-group', members: group_members) } + + let(:catalog) do + Puppet::Resource::Catalog.new.tap do |cat| + cat.add_resource(group) + end + end + + before(:each) do + allow(Puppet::Util::Storage).to receive(:store) + + # Allows retrieving groups + stub_request(:get, 'http://pulpcore-api/pulp/api/v3/groups/?limit=100&offset=0') + .to_return( + status: 200, + body: File.read('spec/fixtures/pulp_data/get_groups.json'), + ) + + # Allow retrieving members assigned to the groups + stub_request(:get, 'http://pulpcore-api/pulp/api/v3/groups/1/users/?limit=100&offset=0') + .to_return( + status: 200, + body: File.read('spec/fixtures/pulp_data/get_group_users.json'), + ) + end + + describe 'with no differences' do + it 'does nothing' do + report = catalog.apply.report + report.finalize_report + expect(report.logs).to be_empty + expect(report.status).to eq('unchanged') + end + end + + describe 'with ordering difference' do + let(:group_members) { ['example-user2', 'example-user1'] } + + it 'performs no changes' do + report = catalog.apply.report + report.finalize_report + expect(report.logs).to be_empty + expect(report.status).to eq('unchanged') + end + end + + describe 'with content difference' do + let(:group_members) { ['example-user'] } + + it 'performs changes' do + del_req1 = stub_request(:delete, 'http://pulpcore-api/pulp/api/v3/groups/1/users/5/') + .to_return( + status: 200, + ) + del_req2 = stub_request(:delete, 'http://pulpcore-api/pulp/api/v3/groups/1/users/6/') + .to_return( + status: 200, + ) + add_req = stub_request(:post, 'http://pulpcore-api/pulp/api/v3/groups/1/users/') + .with( + body: { + username: 'example-user', + }, + ) + .to_return( + status: 200, + ) + + report = catalog.apply.report + report.finalize_report + expect(report.status).to eq('changed') + + assert_requested del_req1 + assert_requested del_req2 + assert_requested add_req + end + end +end diff --git a/spec/spec_helper_local.rb b/spec/spec_helper_local.rb index 2cf3d2f73cbbab9180e1d7e9da6133774af819e7..299e8c21f9325428305c8ce85564d04a802669d3 100644 --- a/spec/spec_helper_local.rb +++ b/spec/spec_helper_local.rb @@ -2,6 +2,7 @@ require 'webmock/rspec' require_relative '../lib/puppet_x/liu_pulpcore/connection' +require_relative '../lib/puppet_x/liu_pulpcore/resource_cache' def enable_pulp3_testing PuppetX::LiuPulpcore::Connection.class_eval do @@ -9,6 +10,23 @@ def enable_pulp3_testing end end +def drop_pulp3_caches + pulpcore_types = [:user, :group, :role] + [:deb, :rpm, :python].each do |plugin| + [:distribution, :publication, :remote, :repository].each do |type| + pulpcore_types << :"#{plugin}_#{type}" + end + end + pulpcore_types.map! { |type| :"liu_pulpcore_#{type}" } + + pulpcore_types.each do |type| + provider = Puppet::Type.type(type).my_provider + + provider.instance_variable_set :@cached_get, nil if provider.instance_variable_defined? :@cached_get + provider.instance_variable_set :@cache, nil if provider.instance_variable_defined? :@cache + end +end + def enable_pulp3_integration PuppetX::LiuPulpcore::Connection.class_eval do @config = nil @@ -124,3 +142,9 @@ def allow_searching_full_cache end enable_pulp3_testing + +RSpec.configure do |config| + config.before(:each) do + drop_pulp3_caches + end +end diff --git a/spec/unit/puppet/provider/liu_pulpcore_group/liu_pulpcore_group_spec.rb b/spec/unit/puppet/provider/liu_pulpcore_group/liu_pulpcore_group_spec.rb index 3b9fc15bc705d8e3e5c9ac56909ab16cb0e168f7..8edeb680a71708bb0e3650fea4cf5c9efb7578da 100644 --- a/spec/unit/puppet/provider/liu_pulpcore_group/liu_pulpcore_group_spec.rb +++ b/spec/unit/puppet/provider/liu_pulpcore_group/liu_pulpcore_group_spec.rb @@ -15,12 +15,17 @@ RSpec.describe Puppet::Provider::LiuPulpcoreGroup::LiuPulpcoreGroup do ensure: 'present', name: 'example-group', members: [ - 'example-user', + 'example-user1', + 'example-user2', ], pulp_href: '/pulp/api/v3/groups/1/', raw_members: [ { - 'username' => 'example-user', + 'username' => 'example-user2', + 'pulp_href' => '/pulp/api/v3/users/6/', + }, + { + 'username' => 'example-user1', 'pulp_href' => '/pulp/api/v3/users/5/', }, ], @@ -66,21 +71,32 @@ RSpec.describe Puppet::Provider::LiuPulpcoreGroup::LiuPulpcoreGroup do pulp_href: '/pulp/api/v3/groups/1/' }.to_json, ) - req_member = stub_request(:post, 'http://pulpcore-api/pulp/api/v3/groups/1/users/') - .with( - body: { - username: 'example-user', - }, - ) - .to_return( - status: 200, - body: '{}', - ) + req_member1 = stub_request(:post, 'http://pulpcore-api/pulp/api/v3/groups/1/users/') + .with( + body: { + username: 'example-user1', + }, + ) + .to_return( + status: 200, + body: '{}', + ) + req_member2 = stub_request(:post, 'http://pulpcore-api/pulp/api/v3/groups/1/users/') + .with( + body: { + username: 'example-user2', + }, + ) + .to_return( + status: 200, + body: '{}', + ) provider.create(context, test_data[:name], test_data) assert_requested req - assert_requested req_member + assert_requested req_member1 + assert_requested req_member2 end end @@ -102,13 +118,25 @@ RSpec.describe Puppet::Provider::LiuPulpcoreGroup::LiuPulpcoreGroup do body: '{}', ) - test_data[:members] = ['another-user'] + test_data[:members] = ['another-user', 'example-user2'] provider.update(context, test_data[:name], test_data) assert_requested user_add_req assert_requested user_del_req end + + it 'ignores user ordering' do + user_add_req = stub_request(:post, 'http://pulpcore-api/pulp/api/v3/groups/1/users/') + user_del_req = stub_request(:delete, %r{http://pulpcore-api/pulp/api/v3/groups/1/users/\d+/}) + + test_data[:members] = ['example-user2', 'example-user1'] + + provider.update(context, test_data[:name], test_data) + + refute_requested user_add_req + refute_requested user_del_req + end end describe 'delete(context, name)' do