Repository: incubator-trafficcontrol Updated Branches: refs/heads/master a1bf58f85 -> d37e3865c
tenant utils - depth, height and relations. including temp addition to the API for simple testing Project: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/commit/ed169cd8 Tree: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/tree/ed169cd8 Diff: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/diff/ed169cd8 Branch: refs/heads/master Commit: ed169cd8b2e2d2152842b73f03ee513b0b5389b4 Parents: 16eb259 Author: nir-sopher <[email protected]> Authored: Sun Jun 4 20:05:38 2017 +0300 Committer: Jeremy Mitchell <[email protected]> Committed: Tue Jul 18 12:12:31 2017 -0600 ---------------------------------------------------------------------- traffic_ops/app/lib/API/Tenant.pm | 70 ++++++++++++++-- traffic_ops/app/lib/UI/TenantUtils.pm | 128 ++++++++++++++++++++++++----- traffic_ops/app/t/api/1.2/tenant.t | 73 ++++++++++++++++ 3 files changed, 247 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/ed169cd8/traffic_ops/app/lib/API/Tenant.pm ---------------------------------------------------------------------- diff --git a/traffic_ops/app/lib/API/Tenant.pm b/traffic_ops/app/lib/API/Tenant.pm index e9b9cb5..f224967 100644 --- a/traffic_ops/app/lib/API/Tenant.pm +++ b/traffic_ops/app/lib/API/Tenant.pm @@ -45,11 +45,13 @@ sub index { if ($tenant_utils->is_tenant_resource_readable($tenants_data, $row->id)) { push( @data, { - "id" => $row->id, - "name" => $row->name, - "active" => \$row->active, - "parentId" => $row->parent_id, - "parentName" => ( defined $row->parent_id ) ? $tenant_utils->get_tenant($tenants_data, $row->parent_id)->name : undef, + "id" => $row->id, + "name" => $row->name, + "active" => \$row->active, + "parentId" => $row->parent_id, + "parentName" => ( defined $row->parent_id ) ? $tenant_utils->get_tenant($tenants_data, $row->parent_id)->name : undef, + "heirarchyDepth" => $tenant_utils->get_tenant_heirarchy_depth($tenants_data, $row->id), + "heirarchyHeight" => $tenant_utils->get_tenant_heirarchy_height($tenants_data, $row->id), } ); } @@ -76,6 +78,8 @@ sub show { "active" => \$row->active, "parentId" => $row->parent_id, "parentName" => ( defined $row->parent_id ) ? $tenant_utils->get_tenant($tenants_data, $row->parent_id)->name : undef, + "heirarchyDepth" => $tenant_utils->get_tenant_heirarchy_depth($tenants_data, $row->id), + "heirarchyHeight" => $tenant_utils->get_tenant_heirarchy_height($tenants_data, $row->id), } ); } @@ -148,6 +152,46 @@ sub update { } + if ($params->{parentId} != $tenant->parent) { + #parent replacement + if (!defined($tenant_utils->get_tenant($tenants_data, $params->{parentId}))) { + return $self->alert("Parent tenant does not exists."); + } + my $parent_depth = $tenant_utils->get_tenant_heirarchy_depth($tenants_data, $params->{parentId}); + if (!defined($parent_depth)) + { + return $self->alert("Failed to retrieve parent tenant depth."); + } + + my $tenant_height = $tenant_utils->get_tenant_heirarchy_height($tenants_data, $id); + if (!defined($tenant_height)) + { + return $self->alert("Failed to retrieve tenant height."); + } + + if ($parent_depth+$tenant_height+1 > $tenant_utils->max_heirarchy_limit()) + { + return $self->alert("Parent tenant is invalid: heirarchy limit reached."); + } + + if ($params->{parentId} == $id){ + return $self->alert("Parent tenant is invalid: same as updated tenant."); + } + + my $is_tenant_achestor_of_parent = $tenant_utils->is_anchestor_of($tenants_data, $id, $params->{parentId}); + if (!defined($is_tenant_achestor_of_parent)) + { + return $self->alert("Failed to check tenant and parent current relations."); + } + + if ($is_tenant_achestor_of_parent) + { + return $self->alert("Parent tenant is invalid: a child of the updated tenant."); + } + + } + + #operation my $values = { name => $params->{name}, @@ -209,6 +253,22 @@ sub create { return $self->alert("Parent tenant to be set is not under user's tenancy."); } + if (!defined($tenant_utils->get_tenant($tenants_data, $params->{parentId}))) { + return $self->alert("Parent tenant does not exists."); + } + + my $parent_depth = $tenant_utils->get_tenant_heirarchy_depth($tenants_data, $params->{parentId}); + + if (!defined($parent_depth)) + { + return $self->alert("Failed to retrieve parent tenant depth."); + } + + if ($parent_depth+1 > $tenant_utils->max_heirarchy_limit()-1) + { + return $self->alert("Parent tenant is invalid: heirarchy limit reached."); + } + my $existing = $self->db->resultset('Tenant')->search( { name => $name } )->get_column('name')->single(); if ($existing) { return $self->alert("A tenant with name \"$name\" already exists."); http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/ed169cd8/traffic_ops/app/lib/UI/TenantUtils.pm ---------------------------------------------------------------------- diff --git a/traffic_ops/app/lib/UI/TenantUtils.pm b/traffic_ops/app/lib/UI/TenantUtils.pm index 6b02f8e..4c8fc0f 100644 --- a/traffic_ops/app/lib/UI/TenantUtils.pm +++ b/traffic_ops/app/lib/UI/TenantUtils.pm @@ -170,15 +170,116 @@ sub is_tenant_resource_writeable { return $self->_is_resource_accessable ($tenants_data, $resource_tenancy, "w"); } +sub get_tenant_heirarchy_depth { + #return "undef" in case of error + #a root tenant is of depth 0 + my $self = shift; + my $tenants_data = shift; + my $tenant_id = shift; + if (!defined($tenants_data->{tenants_dict}{$tenant_id})) { + return undef; #tenant does not exists #TODO -ask jeremy how to log + } -############################################################## + my $iter_id = $tenant_id; + + my $depth = 0; + while (defined($iter_id)) { + $iter_id = $tenants_data->{tenants_dict}{$iter_id}{parent}; + $depth++; + if ($depth > $self->max_heirarchy_limit()) + { + return undef; #heirarchy limit #TODO -ask jeremy how to log + } + } + + return $depth-1; +} + +sub get_tenant_heirarchy_height { + #return "undef" in case of error + #a leaf tenant is of height 0 + my $self = shift; + my $tenants_data = shift; + my $tenant_id = shift; + + if (!defined($tenants_data->{tenants_dict}{$tenant_id})) { + return undef; #tenant does not exists #TODO -ask jeremy how to log + } + + #calc tenant height + my @tenants_list = reverse($self->get_hierarchic_tenants_list($tenants_data, $tenant_id)); + my %tenants_height = {}; + + foreach my $tenant_row (@tenants_list) { + my $tid = $tenant_row->id; + $tenants_height{$tid} = 0; + } + + foreach my $tenant_row (@tenants_list) { + my $tid = $tenant_row->id; + my $par_id = $tenant_row->parent_id; + if (($tenants_height{$par_id}) < ($tenants_height{$tid}+1)) { + $tenants_height{$par_id} = $tenants_height{$tid}+1; + } + } + + return $tenants_height{$tenant_id}; +} + + +sub is_anchestor_of { + #return "undef" in case of error + my $self = shift; + my $tenants_data = shift; + my $anchestor_id = shift; + my $descendant_id = shift; + + if (!defined($anchestor_id)) { + return undef; #anchestor tenant is not defined #TODO -ask jeremy how to log + } + + if (!defined($tenants_data->{tenants_dict}{$anchestor_id})) { + return undef; #anchestor tenant does not exists #TODO -ask jeremy how to log + } + + if (!defined($descendant_id)) { + return undef; #descendant tenant is not defined #TODO -ask jeremy how to log + } + + if (!defined($tenants_data->{tenants_dict}{$descendant_id})) { + return undef; #descendant tenant does not exists #TODO -ask jeremy how to log + } + + my $iter_id = $descendant_id; + + my $descendant_depth = 0; + while (defined($iter_id)) { + if ($anchestor_id == $iter_id) + { + return 1; + } + $iter_id = $tenants_data->{tenants_dict}{$iter_id}{parent}; + $descendant_depth++; + if ($descendant_depth > $self->max_heirarchy_limit()) + {#recursion limit + return undef; #TODO -ask jeremy how to log + } + } + + return 0; +} -sub _tenancy_heirarchy_limit { +sub max_heirarchy_limit { my $self = shift; return 100; } + + + +############################################################## + sub _is_resource_accessable { my $self = shift; my $tenants_data = shift; @@ -219,23 +320,12 @@ sub _is_resource_accessable { } #checking if the user tenant is an ancestor of the resource tenant - for (my $depth = 0; $depth < $self->_tenancy_heirarchy_limit(); $depth++) { - - if (!defined($resource_tenant)){ - #reached top tenant, resource is not under the user tenancy - return 0; - } - - if ($user_tenant == $resource_tenant) { - #resource has child tenancy of the user, operations are allowed - return 1; - } - - $resource_tenant = $tenants_data->{tenants_dict}->{$resource_tenant}->{parent}; - }; - - #not found - recursion limit, give only access to root tenant - return $self->is_root_tenant($tenants_data, $user_tenant); + my $is_user_tenat_parent_of_resource = $self->is_anchestor_of($tenants_data, $user_tenant, $resource_tenant); + if (!defined($is_user_tenat_parent_of_resource)) { + #error - give access only to root tenant (so it can fix the problem) + return $self->is_root_tenant($tenants_data, $user_tenant); + } + return $is_user_tenat_parent_of_resource; } 1; http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/ed169cd8/traffic_ops/app/t/api/1.2/tenant.t ---------------------------------------------------------------------- diff --git a/traffic_ops/app/t/api/1.2/tenant.t b/traffic_ops/app/t/api/1.2/tenant.t index c2662b3..903b2f1 100644 --- a/traffic_ops/app/t/api/1.2/tenant.t +++ b/traffic_ops/app/t/api/1.2/tenant.t @@ -172,6 +172,79 @@ $t->get_ok("/api/1.2/tenants")->status_is(200) ->json_is( "/response/4/id", $tenantE_id) ->json_is( "/response/1/id", $tenantB_id)->or( sub { diag $t->tx->res->content->asset->{content}; } );; +#tenants heirarchy- test depth and height +$t->get_ok("/api/1.2/tenants/$root_tenant_id")->status_is(200) + ->json_is( "/response/0/heirarchyDepth", 0) + ->json_is( "/response/0/heirarchyHeight", 2) + ->or( sub { diag $t->tx->res->content->asset->{content}; } );; + +$t->get_ok("/api/1.2/tenants/$tenantA_id")->status_is(200) + ->json_is( "/response/0/heirarchyDepth", 1) + ->json_is( "/response/0/heirarchyHeight", 1) + ->or( sub { diag $t->tx->res->content->asset->{content}; } );; + +$t->get_ok("/api/1.2/tenants/$tenantB_id")->status_is(200) + ->json_is( "/response/0/heirarchyDepth", 1) + ->json_is( "/response/0/heirarchyHeight", 0) + ->or( sub { diag $t->tx->res->content->asset->{content}; } );; + +$t->get_ok("/api/1.2/tenants/$tenantD_id")->status_is(200) + ->json_is( "/response/0/heirarchyDepth", 2) + ->json_is( "/response/0/heirarchyHeight", 0) + ->or( sub { diag $t->tx->res->content->asset->{content}; } );; + +$t->get_ok("/api/1.2/tenants/$tenantE_id")->status_is(200) + ->json_is( "/response/0/heirarchyDepth", 2) + ->json_is( "/response/0/heirarchyHeight", 0) + ->or( sub { diag $t->tx->res->content->asset->{content}; } );; + + +#moving A to be the child of B +ok $t->put_ok('/api/1.2/tenants/' . $tenantA_id => {Accept => 'application/json'} => json => { + "active" => 1, "parentId" => $tenantB_id, name => "tenantA2"}) + ->status_is(200); + +$t->get_ok("/api/1.2/tenants/$tenantB_id")->status_is(200) + ->json_is( "/response/0/heirarchyDepth", 2) + ->json_is( "/response/0/heirarchyHeight", 3) + ->or( sub { diag $t->tx->res->content->asset->{content}; } );; + +$t->get_ok("/api/1.2/tenants/$tenantA_id")->status_is(200) + ->json_is( "/response/0/parentId", $tenantB_id) + ->json_is( "/response/0/heirarchyDepth", 3) + ->json_is( "/response/0/heirarchyHeight", 2) + ->or( sub { diag $t->tx->res->content->asset->{content}; } );; + +$t->get_ok("/api/1.2/tenants/$tenantD_id")->status_is(200) + ->json_is( "/response/0/parentId", $tenantA_id) + ->json_is( "/response/0/heirarchyDepth", 4) + ->json_is( "/response/0/heirarchyHeight", 1) + ->or( sub { diag $t->tx->res->content->asset->{content}; } );; + + +#Cannot move B to be the child of itself +ok $t->put_ok('/api/1.2/tenants/' . $tenantB_id => {Accept => 'application/json'} => json => { + "active" => 1, "parentId" => $tenantB_id, name => "tenant"}) + ->json_is( "/alerts/0/text" => "Parent tenant is invalid: same as updated tenant.") + ->status_is(400); + +#Cannot move B to be the child of A (a descendant) +ok $t->put_ok('/api/1.2/tenants/' . $tenantB_id => {Accept => 'application/json'} => json => { + "active" => 1, "parentId" => $tenantA_id, name => "tenant"}) + ->json_is( "/alerts/0/text" => "Parent tenant is invalid: a child of the updated tenant.") + ->status_is(400); + +#move A back +ok $t->put_ok('/api/1.2/tenants/' . $tenantA_id => {Accept => 'application/json'} => json => { + "active" => 1, "parentId" => $root_tenant_id, name => "tenantA2"}) + ->status_is(200); + +$t->get_ok("/api/1.2/tenants/$tenantA_id")->status_is(200) + ->json_is( "/response/0/parentId", $root_tenant_id) + ->json_is( "/response/0/heirarchyDepth", 2) + ->json_is( "/response/0/heirarchyHeight", 2) + ->or( sub { diag $t->tx->res->content->asset->{content}; } );; + #cannot delete a tenant that have children ok $t->delete_ok('/api/1.2/tenants/' . $tenantA_id)->status_is(400) ->json_is( "/alerts/0/text" => "Tenant 'tenantA2' has children tenant(s): e.g 'tenantD'. Please update these tenants and retry." )
