Fix incorrectly tenant default groups #278
Conversation
|
@kbrock Cannot apply the following label because they are not recognized: bug hammer/yes |
| migrate | ||
|
|
||
| t.reload | ||
| expect(t.default_miq_group_id).to be_truthy |
There was a problem hiding this comment.
This feels like an unnecessary expectation since you're going to use that column in the next expectation
| @@ -0,0 +1,56 @@ | |||
| class FixDefaultTenantGroup < ActiveRecord::Migration[5.0] | |||
| class Tenant < ActiveRecord::Base | |||
| belongs_to :default_miq_group, :class_name => "::FixDefaultTenantGroup::MiqGroup", :dependent => :destroy | |||
There was a problem hiding this comment.
I don't see the :dependent => :destroy used anywhere in the migration
|
thnx @bdunne Made those changes |
|
|
||
| def up | ||
| say_with_time "adding default tenant groups" do | ||
| Tenant.all.reject { |t| t.default_miq_group&.tenant_group? }.each do |t| |
There was a problem hiding this comment.
Should we avoid looping through the list twice with something like this?
Tenant.find_each do |t|
next if t.default_miq_group&.tenant_group?
t.default_miq_group_id = nil
t.add_default_miq_group
end
When creating a tenant, if there are already groups in the database, it would assign a random group. This would manifest by having a seemingly random group assigned to newly discovered VMs/Hosts. There can then be privilege issues as the admins possibly don't have access to the group. This only happens for customers who had a database before tenancy was introduced in 2015. It incorrectly assigned during the migration process in 20151021174140_assign_tenant_default_group This also will probably only happen for the default tenant. The fix: When a tenant points to a group that is not a tenant group, change the default group to point to a new tenant group. The old group was an incorrect group. https://bugzilla.redhat.com/show_bug.cgi?id=1625788 I also changed the MiqGroup code to choose the correct group.
|
Checked commit kbrock@fb30ad6 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
|
@simaishi Since this is a data migration only (no schema changes), it should be okay to backport to hammer. Also, users with a new deployment from the hammer branch shouldn't be affected since the PR to the core repo would prevent them from getting into this situation in the first place. |
Fix incorrectly tenant default groups (cherry picked from commit 46cc3f2)
|
Hammer backport details: |
When creating a tenant, if there are already groups in the database,
it would assign a random group.
This would manifest by having a seemingly random group assigned to newly
discovered VMs/Hosts. There can then be privilege issues as the admins
possibly don't have access to the group.
This only happens for customers who had a database before tenancy
was introduced in 2015. It incorrectly assigned during the migration
process in 20151021174140_assign_tenant_default_group
This also will probably only happen for the default tenant.
The fix: When a tenant points to a group that is not a tenant group,
change the default group to point to a new tenant group.
The old group was an incorrect group.
https://bugzilla.redhat.com/show_bug.cgi?id=1625788
I also changed the MiqGroup code to choose the correct group.
see also ManageIQ/manageiq#18025
@miq-bot add_label bug hammer/yes
@miq-bot assign @Fryguy