[GitHub] incubator-trafficcontrol issue #644: Tenant utils

2017-06-28 Thread nir-sopher 
Github user nir-sopher commented on the issue:

https://github.com/apache/incubator-trafficcontrol/pull/644
  
True.
Note that there is also a matter of consistency: Tenant update can also be
done only by the parent. Would you like to change that as well?

On Jun 29, 2017 12:23 AM, "Jeremy Mitchell" 
wrote:

> *@mitchell852* commented on this pull request.
> --
>
> In traffic_ops/app/lib/API/Tenant.pm
> 

> :
>
> > @@ -248,7 +326,17 @@ sub delete {
>   if ( !defined($tenant) ) {
>   return $self->not_found();
>   }   
> - my $name = $self->db->resultset('Tenant')->search( { id => $id } 
)->get_column('name')->single();
> +
> + my $parent_tenant = $tenant->parent_id; 
> + 
> + my $tenant_utils = UI::TenantUtils->new($self);
> + my $tenants_data = $tenant_utils->create_tenants_data_from_db(undef);
> + 
> + if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, 
$parent_tenant)) {
>
> oh, that's a good point. if i try to delete my own tenant, i will get back
> "sorry, this tenant is assigned to a user", right?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> 
,
> or mute the thread
> 

> .
>



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TC-405) copyrights missing on some files

2017-06-28 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/TC-405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16067557#comment-16067557
 ] 

ASF GitHub Bot commented on TC-405:
---

GitHub user dangogh opened a pull request:

https://github.com/apache/incubator-trafficcontrol/pull/704

[TC-405] add license to .md files; remove copyright line from a few files



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/dangogh/incubator-trafficcontrol 
more-copyright

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/incubator-trafficcontrol/pull/704.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #704


commit ddb70a46e710fdbaa6e6bf394a4d3b30e79c5af1
Author: Dan Kirkwood 
Date:   2017-06-28T23:50:21Z

remove copyright

commit 10e8e14f43aca71c3e67e7029cd5bbbffba02e8f
Author: Dan Kirkwood 
Date:   2017-06-28T23:50:27Z

don't ignore .md files for license

commit 8ba4ba6030f807b6eb28d556c972be3df8a0532c
Author: Dan Kirkwood 
Date:   2017-06-28T23:52:04Z

don't ignore .md files for license requirement




> copyrights missing on some files
> 
>
> Key: TC-405
> URL: https://issues.apache.org/jira/browse/TC-405
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: all
>Affects Versions: 2.0.0
>Reporter: Dan Kirkwood
>Assignee: Dan Kirkwood
> Fix For: 2.1.0
>
>
> These files are missing copyright headers:
> * traffic_ops/app/t/api/1.1/cdn.t
> * traffic_ops/client/delivery_service_resources.go
> * traffic_ops/install/lib/WebDep.pm
> * traffic_stats/vendor/github.com/jheitz200/test_helper/http.go
> * traffic_stats/vendor/github.com/jheitz200/test_helper/test.go
> and markdown files (.md) should also carry the copyright header:
> * ./BUILD.md
> * ./CONTRIBUTING.md
> * ./README.md
> * ./build/README.md
> * ./infrastructure/docker/README.md
> * ./infrastructure/docker/build/README.md
> * ./infrastructure/test/README.md
> * ./misc/git/README.md
> * ./test/router/Readme.md
> * ./test/router/dnssec/Readme.md
> * ./test/traffic_ops_cfg/Readme.md
> * ./traffic_monitor/README.md
> * ./traffic_monitor_golang/traffic_monitor/README.md
> * ./traffic_ops/INSTALL.md
> * ./traffic_ops/build/README.md
> * ./traffic_ops/client/tests/integration/Readme.md
> * ./traffic_ops/experimental/auth/README.md
> * ./traffic_ops/experimental/server/README.md
> * ./traffic_ops/experimental/ui/README.md
> * ./traffic_ops/experimental/ui/build/README.md
> * ./traffic_ops/experimental/webfront/README.md
> * ./traffic_ops/goto/README.md
> * ./traffic_ops_db/pg-migration/README.md
> * ./traffic_portal/README.md
> * ./traffic_portal/build/README.md
> * ./traffic_router/core/src/test/resources/Readme.md
> * ./traffic_router/neustar/README.md
> * ./traffic_server/README.md
> * ./traffic_server/plugins/astats_over_http/README.md



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Resolved] (TC-278) postinstall -defaults should not run carton

2017-06-28 Thread Dan Kirkwood (JIRA) 

 [ 
https://issues.apache.org/jira/browse/TC-278?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Dan Kirkwood resolved TC-278.
-
Resolution: Fixed

> postinstall -defaults should not run carton
> ---
>
> Key: TC-278
> URL: https://issues.apache.org/jira/browse/TC-278
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: Traffic Ops
>Affects Versions: 2.1.0
>Reporter: Dan Kirkwood
>Assignee: Dan Kirkwood
>  Labels: carton, postinstall
> Fix For: 2.1.0
>
>
> `postinstall -defaults` just writes out a .json file with the default values 
> for customization.   It shouldn't try to do anything else (e.g. check 
> postgres installation, check for root, run carton, etc..)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Resolved] (TC-228) postinstall changes needed for postgres

2017-06-28 Thread Dan Kirkwood (JIRA) 

 [ 
https://issues.apache.org/jira/browse/TC-228?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Dan Kirkwood resolved TC-228.
-
Resolution: Fixed

> postinstall changes needed for postgres
> ---
>
> Key: TC-228
> URL: https://issues.apache.org/jira/browse/TC-228
> Project: Traffic Control
>  Issue Type: Bug
>  Components: Traffic Ops
>Affects Versions: 2.0.0
>Reporter: Dan Kirkwood
>Assignee: Dan Kirkwood
>  Labels: postgres, postinstall
> Fix For: 2.0.0
>
>
> more changes needed to get postinstall working correctly for a postgres 
> installation.
> This is needed for 2.0.x release.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Assigned] (TC-405) copyrights missing on some files

2017-06-28 Thread Dan Kirkwood (JIRA) 

 [ 
https://issues.apache.org/jira/browse/TC-405?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Dan Kirkwood reassigned TC-405:
---

Assignee: Dan Kirkwood

> copyrights missing on some files
> 
>
> Key: TC-405
> URL: https://issues.apache.org/jira/browse/TC-405
> Project: Traffic Control
>  Issue Type: Improvement
>  Components: all
>Affects Versions: 2.0.0
>Reporter: Dan Kirkwood
>Assignee: Dan Kirkwood
> Fix For: 2.1.0
>
>
> These files are missing copyright headers:
> * traffic_ops/app/t/api/1.1/cdn.t
> * traffic_ops/client/delivery_service_resources.go
> * traffic_ops/install/lib/WebDep.pm
> * traffic_stats/vendor/github.com/jheitz200/test_helper/http.go
> * traffic_stats/vendor/github.com/jheitz200/test_helper/test.go
> and markdown files (.md) should also carry the copyright header:
> * ./BUILD.md
> * ./CONTRIBUTING.md
> * ./README.md
> * ./build/README.md
> * ./infrastructure/docker/README.md
> * ./infrastructure/docker/build/README.md
> * ./infrastructure/test/README.md
> * ./misc/git/README.md
> * ./test/router/Readme.md
> * ./test/router/dnssec/Readme.md
> * ./test/traffic_ops_cfg/Readme.md
> * ./traffic_monitor/README.md
> * ./traffic_monitor_golang/traffic_monitor/README.md
> * ./traffic_ops/INSTALL.md
> * ./traffic_ops/build/README.md
> * ./traffic_ops/client/tests/integration/Readme.md
> * ./traffic_ops/experimental/auth/README.md
> * ./traffic_ops/experimental/server/README.md
> * ./traffic_ops/experimental/ui/README.md
> * ./traffic_ops/experimental/ui/build/README.md
> * ./traffic_ops/experimental/webfront/README.md
> * ./traffic_ops/goto/README.md
> * ./traffic_ops_db/pg-migration/README.md
> * ./traffic_portal/README.md
> * ./traffic_portal/build/README.md
> * ./traffic_router/core/src/test/resources/Readme.md
> * ./traffic_router/neustar/README.md
> * ./traffic_server/README.md
> * ./traffic_server/plugins/astats_over_http/README.md



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Created] (TC-405) copyrights missing on some files

2017-06-28 Thread Dan Kirkwood (JIRA) 
Dan Kirkwood created TC-405:
---

 Summary: copyrights missing on some files
 Key: TC-405
 URL: https://issues.apache.org/jira/browse/TC-405
 Project: Traffic Control
  Issue Type: Improvement
  Components: all
Affects Versions: 2.0.0
Reporter: Dan Kirkwood
 Fix For: 2.1.0


These files are missing copyright headers:

* traffic_ops/app/t/api/1.1/cdn.t
* traffic_ops/client/delivery_service_resources.go
* traffic_ops/install/lib/WebDep.pm
* traffic_stats/vendor/github.com/jheitz200/test_helper/http.go
* traffic_stats/vendor/github.com/jheitz200/test_helper/test.go

and markdown files (.md) should also carry the copyright header:
* ./BUILD.md
* ./CONTRIBUTING.md
* ./README.md
* ./build/README.md
* ./infrastructure/docker/README.md
* ./infrastructure/docker/build/README.md
* ./infrastructure/test/README.md
* ./misc/git/README.md
* ./test/router/Readme.md
* ./test/router/dnssec/Readme.md
* ./test/traffic_ops_cfg/Readme.md
* ./traffic_monitor/README.md
* ./traffic_monitor_golang/traffic_monitor/README.md
* ./traffic_ops/INSTALL.md
* ./traffic_ops/build/README.md
* ./traffic_ops/client/tests/integration/Readme.md
* ./traffic_ops/experimental/auth/README.md
* ./traffic_ops/experimental/server/README.md
* ./traffic_ops/experimental/ui/README.md
* ./traffic_ops/experimental/ui/build/README.md
* ./traffic_ops/experimental/webfront/README.md
* ./traffic_ops/goto/README.md
* ./traffic_ops_db/pg-migration/README.md
* ./traffic_portal/README.md
* ./traffic_portal/build/README.md
* ./traffic_router/core/src/test/resources/Readme.md
* ./traffic_router/neustar/README.md
* ./traffic_server/README.md
* ./traffic_server/plugins/astats_over_http/README.md




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread mitchell852 
Github user mitchell852 commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124660517
  
--- Diff: traffic_ops/app/lib/API/Tenant.pm ---
@@ -248,7 +326,17 @@ sub delete {
if ( !defined($tenant) ) {
return $self->not_found();
}   
-   my $name = $self->db->resultset('Tenant')->search( { id => $id } 
)->get_column('name')->single();
+
+   my $parent_tenant = $tenant->parent_id; 
+   
+   my $tenant_utils = UI::TenantUtils->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db(undef);
+   
+   if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, 
$parent_tenant)) {
--- End diff --

oh, that's a good point. if i try to delete my own tenant, i will get back 
"sorry, this tenant is assigned to a user", right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread mitchell852 
Github user mitchell852 commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124660063
  
--- Diff: traffic_ops/app/lib/API/Tenant.pm ---
@@ -131,21 +130,74 @@ sub update {
 
my $is_active = $params->{active};

-   if ( !$params->{active} && $self->isRootTenant($id)) {
-   return $self->alert("Root user cannot be in-active.");
+   if ( !$params->{active} && $tenant_utils->is_root_tenant($tenants_data, 
$id)) {
+   return $self->alert("Root tenant cannot be in-active.");
+   }
+
+   #this is a write operation, allowed only by parents of the tenant 
(which are the owners of the resource of type tenant) 
+   my $current_resource_tenancy = $self->db->resultset('Tenant')->search( 
{ id => $id } )->get_column('parent_id')->single();
+   if (!defined($current_resource_tenancy)) {
+   #no parent - the tenant is its-own owner
+   $current_resource_tenancy = $id;
}

+   if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, 
$current_resource_tenancy)) {
+   return $self->forbidden(); #Current owning tenant is not under 
user's tenancy
+   }
 
-   if ( !defined($params->{parentId}) && !isRootTenant($id) ) {
-   return $self->alert("Only the \"root\" tenant can have no 
parent.");
+   if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, 
$params->{parentId})) {
+   return $self->forbidden(); #Parent tenant to be set is not 
under user's tenancy
}
+
+
+   if ($params->{parentId} != $tenant->parent) {
+   #parent replacement
+   if (!defined($tenant_utils->get_tenant_by_id($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())
--- End diff --

ok, i see


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread mitchell852 
Github user mitchell852 commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124659823
  
--- Diff: traffic_ops/app/lib/API/Tenant.pm ---
@@ -121,7 +115,12 @@ sub update {
}   
}   
 
-   if ( !defined( $params->{parentId}) && !$self->isRootTenant($id) ) {
+   my $tenant_utils = UI::TenantUtils->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db(undef);
+
--- End diff --

yes, i would say the root tenant can't be changed in any way. you can't 
change it's name, or it's active flag or it's parent (which has to be null). 
that is a record in the database that can't be changed at all thru the api.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (TC-184) Tenant Hierarchy Creation

2017-06-28 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/TC-184?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16067165#comment-16067165
 ] 

ASF GitHub Bot commented on TC-184:
---

Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/403#discussion_r124647087
  
--- Diff: docs/source/development/traffic_ops_api/v12/deliveryservice.rst 
---
@@ -168,6 +168,8 @@ Delivery Service
   
+--++--+
   | ``sslKeyVersion``|  int   |

  |
   
+--++--+
+  | ``tenantId`` | int| Owning tenant ID   

  |
--- End diff --

Ready in another PR


> Tenant Hierarchy Creation
> -
>
> Key: TC-184
> URL: https://issues.apache.org/jira/browse/TC-184
> Project: Traffic Control
>  Issue Type: New Feature
>  Components: Traffic Ops, Traffic Ops API
>Reporter: Ryan Durfey
>  Labels: access_control, tenant
>
> Design under discussion here: 
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=68715910
> The requirements below are seed requirements and final design guidance should 
> defer to the evolving wiki page discussion referenced above.
> Overview
> The goal of this system is to create a hierarchical tenancy system which is 
> very simple to understand and administer and is highly expandable so that 
> large organizations with many subsidiaries have the flexibility to create 
> child-tenants to their liking.  
> General Seed Requirements
> 1. Provide a structure to create a hierarchy of Tenants where each Tenant has 
> a single parent-Tenant and can have multiple child-Tenants
> 2. Provide the ability to group multiple Delivery Services, Users, and child- 
> Tenants under each Tenant
> 3. Provide the ability to create at least 10 child-Tenant layers within the 
> system
> 4. Make it easy to assign a User to a single Tenant so that they inherit 
> their permissions to all child-Tenants and Delivery Services. 
> 5. Adding child-Tenants or Delivery Services anywhere in an existing tenant 
> hierarchy does not require re-assigning users.  Users inherit pre-defined 
> permissions to new tenants and services added below their assigned layer in 
> the tenancy tree.
> 6. Allow Users  to be assigned multiple Tenants, each with different Roles.  
> While a user would inherit their access to all services and child Tenants 
> below them, they may need more restrictive access further up the Tenant 
> hierarchy or they may want different access to another branch of the tenant 
> tree.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] incubator-trafficcontrol pull request #403: [TC-184] Org tenancy - delivery ...

2017-06-28 Thread nir-sopher 
Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/403#discussion_r124647087
  
--- Diff: docs/source/development/traffic_ops_api/v12/deliveryservice.rst 
---
@@ -168,6 +168,8 @@ Delivery Service
   
+--++--+
   | ``sslKeyVersion``|  int   |

  |
   
+--++--+
+  | ``tenantId`` | int| Owning tenant ID   

  |
--- End diff --

Ready in another PR


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Created] (TC-404) ./pkg should show where resulting logs and artifacts were placed

2017-06-28 Thread Dan Kirkwood (JIRA) 
Dan Kirkwood created TC-404:
---

 Summary: ./pkg should show where resulting logs and artifacts were 
placed
 Key: TC-404
 URL: https://issues.apache.org/jira/browse/TC-404
 Project: Traffic Control
  Issue Type: Improvement
  Components: all
Affects Versions: 2.0.0
Reporter: Dan Kirkwood
 Fix For: 2.1.0


./pkg without -v option only prints out "Building ..." messages.   At the end,  
it should list the contents of the "dist" directory to make it obvious where 
rpm artifacts and log files were placed.




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread nir-sopher 
Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124640284
  
--- Diff: traffic_ops/app/lib/API/Tenant.pm ---
@@ -248,7 +326,17 @@ sub delete {
if ( !defined($tenant) ) {
return $self->not_found();
}   
-   my $name = $self->db->resultset('Tenant')->search( { id => $id } 
)->get_column('name')->single();
+
+   my $parent_tenant = $tenant->parent_id; 
+   
+   my $tenant_utils = UI::TenantUtils->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db(undef);
+   
+   if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, 
$parent_tenant)) {
--- End diff --

I see the parent as an owner of the child, and as such he is the one to be 
able to delete it.
It is a matter of taste, but note: anyhow a user must not delete his own 
tenant...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread nir-sopher 
Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124639190
  
--- Diff: traffic_ops/app/lib/API/Tenant.pm ---
@@ -131,21 +130,74 @@ sub update {
 
my $is_active = $params->{active};

-   if ( !$params->{active} && $self->isRootTenant($id)) {
-   return $self->alert("Root user cannot be in-active.");
+   if ( !$params->{active} && $tenant_utils->is_root_tenant($tenants_data, 
$id)) {
+   return $self->alert("Root tenant cannot be in-active.");
+   }
+
+   #this is a write operation, allowed only by parents of the tenant 
(which are the owners of the resource of type tenant) 
+   my $current_resource_tenancy = $self->db->resultset('Tenant')->search( 
{ id => $id } )->get_column('parent_id')->single();
+   if (!defined($current_resource_tenancy)) {
+   #no parent - the tenant is its-own owner
+   $current_resource_tenancy = $id;
}

+   if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, 
$current_resource_tenancy)) {
+   return $self->forbidden(); #Current owning tenant is not under 
user's tenancy
+   }
 
-   if ( !defined($params->{parentId}) && !isRootTenant($id) ) {
-   return $self->alert("Only the \"root\" tenant can have no 
parent.");
+   if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, 
$params->{parentId})) {
+   return $self->forbidden(); #Parent tenant to be set is not 
under user's tenancy
}
+
+
+   if ($params->{parentId} != $tenant->parent) {
+   #parent replacement
+   if (!defined($tenant_utils->get_tenant_by_id($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.");
--- End diff --

The above test comes to verify we do not create cyclic graph by parent 
pointing. We check if the designated child is already an ancestor of his 
parent.  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread nir-sopher 
Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124638570
  
--- Diff: traffic_ops/app/lib/API/Tenant.pm ---
@@ -131,21 +130,74 @@ sub update {
 
my $is_active = $params->{active};

-   if ( !$params->{active} && $self->isRootTenant($id)) {
-   return $self->alert("Root user cannot be in-active.");
+   if ( !$params->{active} && $tenant_utils->is_root_tenant($tenants_data, 
$id)) {
+   return $self->alert("Root tenant cannot be in-active.");
+   }
+
+   #this is a write operation, allowed only by parents of the tenant 
(which are the owners of the resource of type tenant) 
+   my $current_resource_tenancy = $self->db->resultset('Tenant')->search( 
{ id => $id } )->get_column('parent_id')->single();
+   if (!defined($current_resource_tenancy)) {
+   #no parent - the tenant is its-own owner
+   $current_resource_tenancy = $id;
}

+   if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, 
$current_resource_tenancy)) {
+   return $self->forbidden(); #Current owning tenant is not under 
user's tenancy
+   }
 
-   if ( !defined($params->{parentId}) && !isRootTenant($id) ) {
-   return $self->alert("Only the \"root\" tenant can have no 
parent.");
+   if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, 
$params->{parentId})) {
+   return $self->forbidden(); #Parent tenant to be set is not 
under user's tenancy
}
+
+
+   if ($params->{parentId} != $tenant->parent) {
+   #parent replacement
+   if (!defined($tenant_utils->get_tenant_by_id($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())
--- End diff --

The limit is there to avoid infinite loops in tenant-utils in case of 
cyclic parent pointing. There is no way I;m aware of to create such loops but 
bugs may prove me wrong:)
As I set a limit in the loops themselves, I wanted to keep the code 
coherent and block the option to create deeper hierarchy.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread nir-sopher 
Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124637268
  
--- Diff: traffic_ops/app/lib/API/Tenant.pm ---
@@ -121,7 +115,12 @@ sub update {
}   
}   
 
-   if ( !defined( $params->{parentId}) && !$self->isRootTenant($id) ) {
+   my $tenant_utils = UI::TenantUtils->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db(undef);
+
--- End diff --

Indeed we agreed that a "root" tenant cannot become not root, and a 
non-root cannot become root. It also cannot be disabled.
Does it mean that a root tenant cannot be changed? I;m not sure but it 
indeed simolify things.
Will do


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread nir-sopher 
Github user nir-sopher commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124635238
  
--- Diff: traffic_ops/app/lib/API/Tenant.pm ---
@@ -31,34 +32,28 @@ sub getTenantName {
return defined($tenant_id) ? $self->db->resultset('Tenant')->search( { 
id => $tenant_id } )->get_column('name')->single() : "n/a";
 }
 
-sub isRootTenant {
-   my $self= shift;
-   my $tenant_id   = shift;
-   return !defined($self->db->resultset('Tenant')->search( { id => 
$tenant_id } )->get_column('parent_id')->single());
-}
-
 sub index {
-   my $self= shift;
-   my @data = ();
-   my %idnames;
+   my $self= shift;
my $orderby = $self->param('orderby') || "name";
 
-   my $rs_data = $self->db->resultset("Tenant")->search();
-   while ( my $row = $rs_data->next ) {
-   $idnames{ $row->id } = $row->name;
-   }
+   my $tenant_utils = UI::TenantUtils->new($self);
--- End diff --

Cool:) Will do.
I was not aware of this dir, just put it next to the "Util.pm"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread mitchell852 
Github user mitchell852 commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124628818
  
--- Diff: traffic_ops/app/lib/API/User.pm ---
@@ -404,7 +406,7 @@ sub current {
my $self = shift;
my @data;
my $current_username = $self->current_user()->{username};
-
+   my $tenantUtils = UI::TenantUtils->new($self);
--- End diff --

this can go away. you are not using this variable.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread mitchell852 
Github user mitchell852 commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124592678
  
--- Diff: traffic_ops/app/lib/API/Tenant.pm ---
@@ -31,34 +32,28 @@ sub getTenantName {
return defined($tenant_id) ? $self->db->resultset('Tenant')->search( { 
id => $tenant_id } )->get_column('name')->single() : "n/a";
 }
 
-sub isRootTenant {
-   my $self= shift;
-   my $tenant_id   = shift;
-   return !defined($self->db->resultset('Tenant')->search( { id => 
$tenant_id } )->get_column('parent_id')->single());
-}
-
 sub index {
-   my $self= shift;
-   my @data = ();
-   my %idnames;
+   my $self= shift;
my $orderby = $self->param('orderby') || "name";
 
-   my $rs_data = $self->db->resultset("Tenant")->search();
-   while ( my $row = $rs_data->next ) {
-   $idnames{ $row->id } = $row->name;
-   }
+   my $tenant_utils = UI::TenantUtils->new($self);
--- End diff --

and then you can just call it Utils::Tenant


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread mitchell852 
Github user mitchell852 commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124624257
  
--- Diff: traffic_ops/app/lib/API/Tenant.pm ---
@@ -131,21 +130,74 @@ sub update {
 
my $is_active = $params->{active};

-   if ( !$params->{active} && $self->isRootTenant($id)) {
-   return $self->alert("Root user cannot be in-active.");
+   if ( !$params->{active} && $tenant_utils->is_root_tenant($tenants_data, 
$id)) {
+   return $self->alert("Root tenant cannot be in-active.");
+   }
+
+   #this is a write operation, allowed only by parents of the tenant 
(which are the owners of the resource of type tenant) 
+   my $current_resource_tenancy = $self->db->resultset('Tenant')->search( 
{ id => $id } )->get_column('parent_id')->single();
+   if (!defined($current_resource_tenancy)) {
+   #no parent - the tenant is its-own owner
+   $current_resource_tenancy = $id;
}

+   if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, 
$current_resource_tenancy)) {
+   return $self->forbidden(); #Current owning tenant is not under 
user's tenancy
+   }
 
-   if ( !defined($params->{parentId}) && !isRootTenant($id) ) {
-   return $self->alert("Only the \"root\" tenant can have no 
parent.");
+   if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, 
$params->{parentId})) {
+   return $self->forbidden(); #Parent tenant to be set is not 
under user's tenancy
}
+
+
+   if ($params->{parentId} != $tenant->parent) {
+   #parent replacement
+   if (!defined($tenant_utils->get_tenant_by_id($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.");
--- End diff --

I don't understand this error message


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread mitchell852 
Github user mitchell852 commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124621754
  
--- Diff: traffic_ops/app/lib/API/Tenant.pm ---
@@ -131,21 +130,74 @@ sub update {
 
my $is_active = $params->{active};

-   if ( !$params->{active} && $self->isRootTenant($id)) {
-   return $self->alert("Root user cannot be in-active.");
+   if ( !$params->{active} && $tenant_utils->is_root_tenant($tenants_data, 
$id)) {
+   return $self->alert("Root tenant cannot be in-active.");
+   }
+
+   #this is a write operation, allowed only by parents of the tenant 
(which are the owners of the resource of type tenant) 
+   my $current_resource_tenancy = $self->db->resultset('Tenant')->search( 
{ id => $id } )->get_column('parent_id')->single();
+   if (!defined($current_resource_tenancy)) {
+   #no parent - the tenant is its-own owner
--- End diff --

every tenant must have a parent, right? except for the root tenant. 
therefore, I don't think this is necessary if  you follow my advice above and 
exit the function if the tenant is the root tenant.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread mitchell852 
Github user mitchell852 commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124597615
  
--- Diff: traffic_ops/app/lib/API/Tenant.pm ---
@@ -121,7 +115,12 @@ sub update {
}   
}   
 
-   if ( !defined( $params->{parentId}) && !$self->isRootTenant($id) ) {
+   my $tenant_utils = UI::TenantUtils->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db(undef);
+
--- End diff --

how about right here adding something like:

if ( is_root_tenant($tenants_data, $id) ) {
return $self->alert("Root tenant cannot be updated.");
}

I think we agreed that the root tenant can't be updated but I could be 
wrong. Anyhow, if you put this code right about here, then you won't have to 
check is_root_tenant() anymore in this function which I think will make your 
code easier to read.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread mitchell852 
Github user mitchell852 commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124625194
  
--- Diff: traffic_ops/app/lib/API/Tenant.pm ---
@@ -185,11 +237,36 @@ sub create {
return $self->alert("Tenant name is required.");
}
 
+   #not allowing to create additional root tenants.
+   #there is no real problem with that, but no real use also
my $parent_id = $params->{parentId};
if ( !defined($parent_id) ) {
return $self->alert("Parent Id is required.");
}
+   
+   my $tenant_utils = UI::TenantUtils->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db(undef);
+   
+   if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, 
$params->{parentId})) {
+   return $self->forbidden(); #Parent tenant to be set is not 
under user's tenancy
+   }
 
+   if (!defined($tenant_utils->get_tenant_by_id($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)
--- End diff --

again, if we get rid of the max_heirarchy_limit, then you can simplify this 
code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread mitchell852 
Github user mitchell852 commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124625956
  
--- Diff: traffic_ops/app/lib/API/Tenant.pm ---
@@ -248,7 +326,17 @@ sub delete {
if ( !defined($tenant) ) {
return $self->not_found();
}   
-   my $name = $self->db->resultset('Tenant')->search( { id => $id } 
)->get_column('name')->single();
+
+   my $parent_tenant = $tenant->parent_id; 
+   
+   my $tenant_utils = UI::TenantUtils->new($self);
+   my $tenants_data = $tenant_utils->create_tenants_data_from_db(undef);
+   
+   if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, 
$parent_tenant)) {
--- End diff --

i don't think that deleting a tenant should matter what the parent tenant 
is. for example, if this is the tenant hierarcy:

- root
-- tenant 1
--- tenant 1a
--- tenant 1b

and i have a user with tenant = tenant 1a, then i should be able to delete 
tenant 1a even though i don't have tenant 1 assigned to me. what do you think?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread mitchell852 
Github user mitchell852 commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124589621
  
--- Diff: traffic_ops/app/lib/API/Tenant.pm ---
@@ -31,34 +32,28 @@ sub getTenantName {
return defined($tenant_id) ? $self->db->resultset('Tenant')->search( { 
id => $tenant_id } )->get_column('name')->single() : "n/a";
 }
 
-sub isRootTenant {
-   my $self= shift;
-   my $tenant_id   = shift;
-   return !defined($self->db->resultset('Tenant')->search( { id => 
$tenant_id } )->get_column('parent_id')->single());
-}
-
 sub index {
-   my $self= shift;
-   my @data = ();
-   my %idnames;
+   my $self= shift;
my $orderby = $self->param('orderby') || "name";
 
-   my $rs_data = $self->db->resultset("Tenant")->search();
-   while ( my $row = $rs_data->next ) {
-   $idnames{ $row->id } = $row->name;
-   }
+   my $tenant_utils = UI::TenantUtils->new($self);
--- End diff --

this doesn't seem like it's in the right place. how about putting 
TenantUtils in the Utils directory?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #644: Tenant utils

2017-06-28 Thread mitchell852 
Github user mitchell852 commented on a diff in the pull request:


https://github.com/apache/incubator-trafficcontrol/pull/644#discussion_r124623700
  
--- Diff: traffic_ops/app/lib/API/Tenant.pm ---
@@ -131,21 +130,74 @@ sub update {
 
my $is_active = $params->{active};

-   if ( !$params->{active} && $self->isRootTenant($id)) {
-   return $self->alert("Root user cannot be in-active.");
+   if ( !$params->{active} && $tenant_utils->is_root_tenant($tenants_data, 
$id)) {
+   return $self->alert("Root tenant cannot be in-active.");
+   }
+
+   #this is a write operation, allowed only by parents of the tenant 
(which are the owners of the resource of type tenant) 
+   my $current_resource_tenancy = $self->db->resultset('Tenant')->search( 
{ id => $id } )->get_column('parent_id')->single();
+   if (!defined($current_resource_tenancy)) {
+   #no parent - the tenant is its-own owner
+   $current_resource_tenancy = $id;
}

+   if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, 
$current_resource_tenancy)) {
+   return $self->forbidden(); #Current owning tenant is not under 
user's tenancy
+   }
 
-   if ( !defined($params->{parentId}) && !isRootTenant($id) ) {
-   return $self->alert("Only the \"root\" tenant can have no 
parent.");
+   if (!$tenant_utils->is_tenant_resource_accessible($tenants_data, 
$params->{parentId})) {
+   return $self->forbidden(); #Parent tenant to be set is not 
under user's tenancy
}
+
+
+   if ($params->{parentId} != $tenant->parent) {
+   #parent replacement
+   if (!defined($tenant_utils->get_tenant_by_id($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())
--- End diff --

can we just get rid of the max_heirarchy_limit? that would get rid of your 
need to figure out depth and height and simplify this code


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Created] (TC-403) Comments/Notes field in UI for configurations

2017-06-28 Thread Jason Tucker (JIRA) 
Jason Tucker created TC-403:
---

 Summary: Comments/Notes field in UI for configurations
 Key: TC-403
 URL: https://issues.apache.org/jira/browse/TC-403
 Project: Traffic Control
  Issue Type: Wish
  Components: Traffic Ops
Reporter: Jason Tucker
Priority: Minor


As a Traffic Ops admin user, there are times that I want to attach additional 
configuration notes, comments, caveats, etc to an object (particularly DS and 
Server configs) so that they are readily available to any admin that may need 
to later update the config. Having an arbitrary text field attached to the 
config that could be used for any extra information would be helpful.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)