You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficcontrol.apache.org by Jeremy Mitchell <mi...@apache.org> on 2019/01/08 15:55:27 UTC
Tenancy no longer optional?
I feel like we discussed this and agreed that moving forward tenancy should
no longer be optional in TP/TO but I can't find the email so maybe it was
just a dream. :) (fyi at Comcast, we've been using tenancy for over 1.5
years i think)
Currently, you can toggle tenancy on/off using a parameter (use_tenancy).
This adds complexity to the code which I think can lead to errors in logic.
Also, if tenancy is turned off it falls back to role checking and/or
ds/user assignment. ds/user assignment isn't even supported in TP and role
checking goes against the idea of roles/capabilities.
Therefore, i'd like to suggest that in the next release of TC (4.0.x?),
tenancy is required and cannot be turned off. Of course, if you still don't
want to use tenancy, you can assign all users/delivery services to the
"root" tenant which in essence turns off tenancy for all users.
Making tenancy required would require stripping out all conditions like
this from the Perl:
https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/lib/API/Job.pm#L179
or simply going to the source of that method and make the necessary code
change to prevent turning it off:
https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/lib/Utils/Tenant.pm#L256
Not sure what's required in the Go to ensure that tenancy can't be turned
off.
Jeremy
Re: [EXTERNAL] Tenancy no longer optional?
Posted by Jeremy Mitchell <mi...@gmail.com>.
So it sounds like from Go remove calls to IsTenancyEnabled() and from Perl
remove calls to use_tenancy() and maybe a migration to delete the
use_tenancy parameter and then it's no longer optional. Also, Rob and I
discussed on the side and this would not have to wait for
roles/capabilities.
Jeremy
On Tue, Jan 8, 2019 at 10:20 AM Robert Butts <ro...@apache.org> wrote:
> > Not sure what's required in the Go to ensure that tenancy can't be turned
> off.
>
> And yes, it's pretty easy in Go, too. Just a matter of removing those query
> bits, and calls to IsTenancyEnabled.
>
>
> On Tue, Jan 8, 2019 at 10:09 AM Rawlin Peters <ra...@gmail.com>
> wrote:
>
> > I believe we essentially agreed to make tenancy required in 3.0 (and
> > have done so via a DB migration), but there is still a dependency on
> > the use_tenancy parameter in the codebase. So until we can go through
> > and fix all those places that check the use_tenancy parameter, we
> > can't really get rid of it altogether. In the meantime though, we
> > could potentially disable PUT/DELETE for that specific parameter in
> > the API so that you can't just "disable tenancy" (air quotes because
> > there are endpoints now that already make the assumption that tenancy
> > is always enabled), but it would be best to just go through and fix
> > the code that depends on that parameter.
> >
> > - Rawlin
> > On Tue, Jan 8, 2019 at 8:59 AM Fieck, Brennan <Brennan_Fieck@comcast.com
> >
> > wrote:
> > >
> > > +1 down with optional tenancy.
> > >
> > > Changes in Go would be similar I think - just remove the check for
> > enabled tenancy and the code that handles the disabled case.
> > > (I'd also like to submit for consideration the idea of re-writing Perl
> > endpoints instead of updating them)
> > > ________________________________________
> > > From: Jeremy Mitchell <mi...@apache.org>
> > > Sent: Tuesday, January 8, 2019 8:55 AM
> > > To: dev@trafficcontrol.apache.org
> > > Subject: [EXTERNAL] Tenancy no longer optional?
> > >
> > > I feel like we discussed this and agreed that moving forward tenancy
> > should
> > > no longer be optional in TP/TO but I can't find the email so maybe it
> was
> > > just a dream. :) (fyi at Comcast, we've been using tenancy for over 1.5
> > > years i think)
> > >
> > > Currently, you can toggle tenancy on/off using a parameter
> (use_tenancy).
> > >
> > > This adds complexity to the code which I think can lead to errors in
> > logic.
> > > Also, if tenancy is turned off it falls back to role checking and/or
> > > ds/user assignment. ds/user assignment isn't even supported in TP and
> > role
> > > checking goes against the idea of roles/capabilities.
> > >
> > > Therefore, i'd like to suggest that in the next release of TC (4.0.x?),
> > > tenancy is required and cannot be turned off. Of course, if you still
> > don't
> > > want to use tenancy, you can assign all users/delivery services to the
> > > "root" tenant which in essence turns off tenancy for all users.
> > >
> > > Making tenancy required would require stripping out all conditions like
> > > this from the Perl:
> > >
> > >
> >
> https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/lib/API/Job.pm#L179
> > >
> > > or simply going to the source of that method and make the necessary
> code
> > > change to prevent turning it off:
> > >
> > >
> >
> https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/lib/Utils/Tenant.pm#L256
> > >
> > > Not sure what's required in the Go to ensure that tenancy can't be
> turned
> > > off.
> > >
> > > Jeremy
> >
>
Re: [EXTERNAL] Tenancy no longer optional?
Posted by Robert Butts <ro...@apache.org>.
> Not sure what's required in the Go to ensure that tenancy can't be turned
off.
And yes, it's pretty easy in Go, too. Just a matter of removing those query
bits, and calls to IsTenancyEnabled.
On Tue, Jan 8, 2019 at 10:09 AM Rawlin Peters <ra...@gmail.com>
wrote:
> I believe we essentially agreed to make tenancy required in 3.0 (and
> have done so via a DB migration), but there is still a dependency on
> the use_tenancy parameter in the codebase. So until we can go through
> and fix all those places that check the use_tenancy parameter, we
> can't really get rid of it altogether. In the meantime though, we
> could potentially disable PUT/DELETE for that specific parameter in
> the API so that you can't just "disable tenancy" (air quotes because
> there are endpoints now that already make the assumption that tenancy
> is always enabled), but it would be best to just go through and fix
> the code that depends on that parameter.
>
> - Rawlin
> On Tue, Jan 8, 2019 at 8:59 AM Fieck, Brennan <Br...@comcast.com>
> wrote:
> >
> > +1 down with optional tenancy.
> >
> > Changes in Go would be similar I think - just remove the check for
> enabled tenancy and the code that handles the disabled case.
> > (I'd also like to submit for consideration the idea of re-writing Perl
> endpoints instead of updating them)
> > ________________________________________
> > From: Jeremy Mitchell <mi...@apache.org>
> > Sent: Tuesday, January 8, 2019 8:55 AM
> > To: dev@trafficcontrol.apache.org
> > Subject: [EXTERNAL] Tenancy no longer optional?
> >
> > I feel like we discussed this and agreed that moving forward tenancy
> should
> > no longer be optional in TP/TO but I can't find the email so maybe it was
> > just a dream. :) (fyi at Comcast, we've been using tenancy for over 1.5
> > years i think)
> >
> > Currently, you can toggle tenancy on/off using a parameter (use_tenancy).
> >
> > This adds complexity to the code which I think can lead to errors in
> logic.
> > Also, if tenancy is turned off it falls back to role checking and/or
> > ds/user assignment. ds/user assignment isn't even supported in TP and
> role
> > checking goes against the idea of roles/capabilities.
> >
> > Therefore, i'd like to suggest that in the next release of TC (4.0.x?),
> > tenancy is required and cannot be turned off. Of course, if you still
> don't
> > want to use tenancy, you can assign all users/delivery services to the
> > "root" tenant which in essence turns off tenancy for all users.
> >
> > Making tenancy required would require stripping out all conditions like
> > this from the Perl:
> >
> >
> https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/lib/API/Job.pm#L179
> >
> > or simply going to the source of that method and make the necessary code
> > change to prevent turning it off:
> >
> >
> https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/lib/Utils/Tenant.pm#L256
> >
> > Not sure what's required in the Go to ensure that tenancy can't be turned
> > off.
> >
> > Jeremy
>
Re: [EXTERNAL] Tenancy no longer optional?
Posted by Rawlin Peters <ra...@gmail.com>.
I believe we essentially agreed to make tenancy required in 3.0 (and
have done so via a DB migration), but there is still a dependency on
the use_tenancy parameter in the codebase. So until we can go through
and fix all those places that check the use_tenancy parameter, we
can't really get rid of it altogether. In the meantime though, we
could potentially disable PUT/DELETE for that specific parameter in
the API so that you can't just "disable tenancy" (air quotes because
there are endpoints now that already make the assumption that tenancy
is always enabled), but it would be best to just go through and fix
the code that depends on that parameter.
- Rawlin
On Tue, Jan 8, 2019 at 8:59 AM Fieck, Brennan <Br...@comcast.com> wrote:
>
> +1 down with optional tenancy.
>
> Changes in Go would be similar I think - just remove the check for enabled tenancy and the code that handles the disabled case.
> (I'd also like to submit for consideration the idea of re-writing Perl endpoints instead of updating them)
> ________________________________________
> From: Jeremy Mitchell <mi...@apache.org>
> Sent: Tuesday, January 8, 2019 8:55 AM
> To: dev@trafficcontrol.apache.org
> Subject: [EXTERNAL] Tenancy no longer optional?
>
> I feel like we discussed this and agreed that moving forward tenancy should
> no longer be optional in TP/TO but I can't find the email so maybe it was
> just a dream. :) (fyi at Comcast, we've been using tenancy for over 1.5
> years i think)
>
> Currently, you can toggle tenancy on/off using a parameter (use_tenancy).
>
> This adds complexity to the code which I think can lead to errors in logic.
> Also, if tenancy is turned off it falls back to role checking and/or
> ds/user assignment. ds/user assignment isn't even supported in TP and role
> checking goes against the idea of roles/capabilities.
>
> Therefore, i'd like to suggest that in the next release of TC (4.0.x?),
> tenancy is required and cannot be turned off. Of course, if you still don't
> want to use tenancy, you can assign all users/delivery services to the
> "root" tenant which in essence turns off tenancy for all users.
>
> Making tenancy required would require stripping out all conditions like
> this from the Perl:
>
> https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/lib/API/Job.pm#L179
>
> or simply going to the source of that method and make the necessary code
> change to prevent turning it off:
>
> https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/lib/Utils/Tenant.pm#L256
>
> Not sure what's required in the Go to ensure that tenancy can't be turned
> off.
>
> Jeremy
Re: [EXTERNAL] Tenancy no longer optional?
Posted by Robert Butts <ro...@apache.org>.
> we discussed this and agreed that moving forward tenancy should no longer
be optional in TP/TO but I can't find the email
I think it's this discussion, subject "Enforcing capability rules"
https://lists.apache.org/thread.html/57e2ea49fe27a14e6be06b8d8d8e9608dd8db5a671a5a47c1905f2f8@%3Cdev.trafficcontrol.apache.org%3E
That discussion seems to have consensus to turn on
Tenancy+Roles+Capabilities permanently in 3.0, with the understanding that
default roles will be created which emulate the old Priv Levels.
But, as that discussion says, I think we need to enable Capabilities at the
same time. Tenancy isn't really very useful without Capabilities being
enforced. We can "turn on tenancy" without them, but we still have to use
the old Priv Levels, which also need to go away.
We have a PR for enforcing Capabilities, we just need to take the time to
clean it up, merge it, and test it:
https://github.com/apache/trafficcontrol/pull/2791
TLDR:
- I'm +1
- I think we already agreed to this
- I think we also need to enable Capabilities at the same time
On Tue, Jan 8, 2019 at 8:59 AM Fieck, Brennan <Br...@comcast.com>
wrote:
> +1 down with optional tenancy.
>
> Changes in Go would be similar I think - just remove the check for enabled
> tenancy and the code that handles the disabled case.
> (I'd also like to submit for consideration the idea of re-writing Perl
> endpoints instead of updating them)
> ________________________________________
> From: Jeremy Mitchell <mi...@apache.org>
> Sent: Tuesday, January 8, 2019 8:55 AM
> To: dev@trafficcontrol.apache.org
> Subject: [EXTERNAL] Tenancy no longer optional?
>
> I feel like we discussed this and agreed that moving forward tenancy should
> no longer be optional in TP/TO but I can't find the email so maybe it was
> just a dream. :) (fyi at Comcast, we've been using tenancy for over 1.5
> years i think)
>
> Currently, you can toggle tenancy on/off using a parameter (use_tenancy).
>
> This adds complexity to the code which I think can lead to errors in logic.
> Also, if tenancy is turned off it falls back to role checking and/or
> ds/user assignment. ds/user assignment isn't even supported in TP and role
> checking goes against the idea of roles/capabilities.
>
> Therefore, i'd like to suggest that in the next release of TC (4.0.x?),
> tenancy is required and cannot be turned off. Of course, if you still don't
> want to use tenancy, you can assign all users/delivery services to the
> "root" tenant which in essence turns off tenancy for all users.
>
> Making tenancy required would require stripping out all conditions like
> this from the Perl:
>
>
> https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/lib/API/Job.pm#L179
>
> or simply going to the source of that method and make the necessary code
> change to prevent turning it off:
>
>
> https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/lib/Utils/Tenant.pm#L256
>
> Not sure what's required in the Go to ensure that tenancy can't be turned
> off.
>
> Jeremy
>
Re: [EXTERNAL] Tenancy no longer optional?
Posted by "Fieck, Brennan" <Br...@comcast.com>.
+1 down with optional tenancy.
Changes in Go would be similar I think - just remove the check for enabled tenancy and the code that handles the disabled case.
(I'd also like to submit for consideration the idea of re-writing Perl endpoints instead of updating them)
________________________________________
From: Jeremy Mitchell <mi...@apache.org>
Sent: Tuesday, January 8, 2019 8:55 AM
To: dev@trafficcontrol.apache.org
Subject: [EXTERNAL] Tenancy no longer optional?
I feel like we discussed this and agreed that moving forward tenancy should
no longer be optional in TP/TO but I can't find the email so maybe it was
just a dream. :) (fyi at Comcast, we've been using tenancy for over 1.5
years i think)
Currently, you can toggle tenancy on/off using a parameter (use_tenancy).
This adds complexity to the code which I think can lead to errors in logic.
Also, if tenancy is turned off it falls back to role checking and/or
ds/user assignment. ds/user assignment isn't even supported in TP and role
checking goes against the idea of roles/capabilities.
Therefore, i'd like to suggest that in the next release of TC (4.0.x?),
tenancy is required and cannot be turned off. Of course, if you still don't
want to use tenancy, you can assign all users/delivery services to the
"root" tenant which in essence turns off tenancy for all users.
Making tenancy required would require stripping out all conditions like
this from the Perl:
https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/lib/API/Job.pm#L179
or simply going to the source of that method and make the necessary code
change to prevent turning it off:
https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/lib/Utils/Tenant.pm#L256
Not sure what's required in the Go to ensure that tenancy can't be turned
off.
Jeremy