The advantage is not needing to check against the list of viable tenants plus null. Gets rid of edge cases where a tenant of NULL either must be included or excluded from a list. With an actual tenant id, it's explicit.
The other advantage is that I'm aware of cases where the tenant_id is currently NULL in places where it wasn't intended to be, and that could potentially lead to unintended consequences that the current code may not handle. What's the advantage of NULL other than that's what's already in there? On Thu, Jul 12, 2018 at 1:39 PM Chris Lemmons <[email protected]> wrote: > What is the advantage of having an "unassigned" value that means > effectively what null currently means? In the tenancy situation, we've > already got a value that means unassigned: null. And in a non-tenancy > situation, there's no reason to assign anything other than null to the > value, since it's of no use. > On Thu, Jul 12, 2018 at 12:36 PM Jeremy Mitchell <[email protected]> > wrote: > > > > Ok, just so i'm clear. On clean install or upgrade, in seeds.sql (or > > wherever) create a new tenant called "Unassigned" with parent=root. > > > > If use_tenancy = 0 (off): > > > > find all users where tenant = null and set tenant = root > > find all dss where tenant = null and set tenant = root > > find all origins where tenant = null and set tenant = root > > > > ^^ if tenancy is currently disabled, then setting everything to root will > > have no real impact but if/when tenancy is turned on, you'll want to > audit > > the tenants of each resource....or you could leave everything as root > hence > > essentially disabling tenancy even when it's turned on :) > > > > If use_tenancy = 1 (on): > > > > find all users where tenant = null and set tenant = unassigned > > find all dss where tenant = null and set tenant = unassigned > > find all origins where tenant = null and set tenant = unassigned > > > > ^^ this allows you to easily find resources to assign the proper tenant > to. > > > > Part of me thinks, set all null tenants to unassigned regardless of > > whether use_tenancy > > = 0 or 1 > > > > Jeremy > > > > > > > > > > On Thu, Jul 12, 2018 at 12:12 PM, Dan Kirkwood <[email protected]> > wrote: > > > > > Along those lines, there are currently no constraints in the db to > > > enforce that everything has an assigned tenant. I'd like to change > that. > > > > > > Currently, it's possible to have tenancy turned on but still have > NULL > > > tenant_id on a user, deliveryservice, or origin. That leads to > ambiguous > > > situations that can be painful to handle in code. Here's what I'd like > to > > > suggest: > > > > > > If the use_tenancy parameter is OFF (0), set the tenant to ROOT for > any > > > item without an assigned tenant. If ON, create a new tenant > > > ("unassigned"?) and assign any NULL tenant ids to that instead -- this > > > allows the admin to easily find the stray ones and assign them > > > appropriately. Once all those are assigned, we can add the > constraints > > > to the db to enforce tenancy. Then, we can do away with the code > that > > > supports the use_tenancy flag and simplify the Traffic Ops code. > > > > > > Thoughts? > > > -dan > > > > > > On Thu, Jun 21, 2018 at 8:04 AM Dewayne Richardson <[email protected] > > > > > wrote: > > > > > > > +1 I think giving reasonable defaults and making the > role/capabilities > > > > feature available makes for it's awareness as well. > > > > > > > > > > > > -Dew > > > > > > > > On Sat, Jun 16, 2018 at 9:18 PM Dave Neuman <[email protected]> > wrote: > > > > > > > > > I'll admit I am not super close to this problem so I don't > understand > > > it > > > > > well but, from what I have read, I tend to agree with Jeremy. If > we > > > are > > > > > going to have roles/capabilities we need to just incorporate them > into > > > > the > > > > > system and be done with it. We should have able to provide > reasonable > > > > > defaults and reasonable ways for people to do things that makes it > > > > tenable. > > > > > > > > > > On Fri, Jun 15, 2018 at 9:45 AM Jeremy Mitchell < > [email protected] > > > > > > > > > wrote: > > > > > > > > > > > regarding tenancy: > > > > > > > > > > > > @Nir is right. If you don't feel like using tenancy, then you > give > > > all > > > > > your > > > > > > users tenant=root. so tenancy is easy to sidestep if you're not > > > > > interested > > > > > > in using it. > > > > > > > > > > > > regarding roles: > > > > > > > > > > > > here are our current roles: > > > > > > > > > > > > admin (priv level=30) > > > > > > operations (20) > > > > > > portal (15) <-- terrible name for a role btw. probably my fault > > > > > > federation (15) > > > > > > steering (15) > > > > > > ort (11) > > > > > > read-only (10) > > > > > > disallowed (0) > > > > > > > > > > > > ^^ in this world, it is your role's priv level that determines > what > > > you > > > > > > can/cannot access > > > > > > > > > > > > when roles/capabilities are introduced our roles will look like > this: > > > > > > > > > > > > admin (all capabilities) > > > > > > operations (the capabilities required to at a minimum to > reproduce > > > > > current > > > > > > access level of priv level=20. capabilities yet to be determined) > > > > > > portal (the capabilities required to at a minimum to reproduce > > > current > > > > > > access level of priv level=15. capabilities yet to be determined) > > > > > > federation (the capabilities required to at a minimum to > reproduce > > > > > current > > > > > > access level of priv level=15. capabilities yet to be determined) > > > > > > steering (the capabilities required to at a minimum to reproduce > > > > current > > > > > > access level of priv level=15. capabilities yet to be determined) > > > > > > ort (the capabilities required to at a minimum to reproduce > current > > > > > access > > > > > > level of priv level=11. capabilities yet to be determined) > > > > > > read-only (the capabilities required to at a minimum to reproduce > > > > current > > > > > > access level of priv level=10. capabilities yet to be determined) > > > > > > disallowed (no capabilities) > > > > > > > > > > > > ^^ in this world, it is your role's capabilities that determine > what > > > > you > > > > > > can/cannot access. if you're not interested in a "complex > permissions > > > > > > framework" as @Rob calls it :) , you really have to do nothing > and > > > the > > > > > > access level of your users should not change (assuming we define > the > > > > > proper > > > > > > capabilities for each role). > > > > > > > > > > > > sooo...if in TC 3.0, we make both (tenancy and > roles/capabilities) an > > > > > > integral part of the system we can, like rob said, simplify a > bunch > > > of > > > > > > code. it's hard enough to explain access control/permissions to > > > people. > > > > > > having to explain 2 ways is almost impossible imo :) > > > > > > > > > > > > and yes, @Rob shims could be added to TP and/or the TO API to > handle > > > > both > > > > > > ways but again, more complexity=more bugs, etc. IMO we just need > to > > > > move > > > > > > forward towards the new approach and leave the old behind... > > > > > > > > > > > > Jeremy > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Jun 14, 2018 at 11:39 PM, Nir Sopher <[email protected]> > wrote: > > > > > > > > > > > > > Hi, > > > > > > > I'm with Jeremy on that. > > > > > > > The "use_tenancy" flag came to allow simple transition to the > > > > tenanted > > > > > > > world: > > > > > > > On the transition, the TC owner should adjust all tenancies, > most > > > > > > > importantly create a "root tenant" admin, and only then put the > > > > > > use_tenancy > > > > > > > flag on. > > > > > > > > > > > > > > For 3.0 I believe that tenancy should become mandatory - > transition > > > > > > period > > > > > > > done. > > > > > > > A TC that does not need tenants, should just put all elements > under > > > > the > > > > > > > root tenant. No need to maintain two pathes in the code.... > > > > > > > > > > > > > > We can do the same for "role/capability" - adding a global knob > > > just > > > > > for > > > > > > > the transition. > > > > > > > > > > > > > > Nir > > > > > > > > > > > > > > On Thu, Jun 14, 2018 at 9:38 PM, Robert Butts <[email protected]> > > > > wrote: > > > > > > > > > > > > > > > >will the self-service stuff rob butts is working on be > affected > > > in > > > > > any > > > > > > > > way? Will self-service truly be turned off via a parameter? > > > > > > > > > > > > > > > > What I'm working on, change integrity, shouldn't be affected > by > > > > > turning > > > > > > > off > > > > > > > > tenancy/roles/capabilities. > > > > > > > > > > > > > > > > And we shouldn't turn off the API-visible parts of what I'll > be > > > > > doing: > > > > > > DS > > > > > > > > snapshots, server snapshots, etc. > > > > > > > > > > > > > > > > We should be able to turn off the complex permissions system > for > > > > > > > > self-service, while still retaining safe change integrity > via the > > > > > > > timestamp > > > > > > > > system. > > > > > > > > > > > > > > > > >it would be nice if TC 3.0 made tenancy / roles & > capabilities a > > > > > > > > requirement > > > > > > > > > > > > > > > > Requiring tenancy/roles/capabilities would certainly make the > > > code > > > > > > nicer, > > > > > > > > but I'm afraid it'll make the software much more difficult to > > > use, > > > > > for > > > > > > > > users who want an internal CDN, and have no need for a > complex > > > > > > > permissions > > > > > > > > framework. > > > > > > > > > > > > > > > > @mitchell852 Is it possible to add GUI shims in Traffic > Portal, > > > > > and/or > > > > > > > API > > > > > > > > helpers, to make the interface pretend like > > > > > tenancy/roles/capabilities > > > > > > > > don't exist? E.g. to grant all permissions and the root > tenant to > > > > all > > > > > > > users > > > > > > > > on user-creation, if the "use_self_service" config flag is > set? > > > So > > > > > all > > > > > > > the > > > > > > > > code can keep working the same way, but people who don't need > > > > > > > > tenancy/roles/capabilities can still have the existing > simpler > > > > > > interface? > > > > > > > > How difficult would that be? > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Jun 14, 2018 at 12:18 PM, Jeremy Mitchell < > > > > > > [email protected] > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > I like the idea but what will it really mean to turn off > > > > > > > > use_self_service. > > > > > > > > > I know it will mean tenancy will be disabled and API > > > permissions > > > > > > won't > > > > > > > be > > > > > > > > > checked against a user's capabilities, but will the > > > self-service > > > > > > stuff > > > > > > > > rob > > > > > > > > > butts is working on be affected in any way? Will > self-service > > > > truly > > > > > > be > > > > > > > > > turned off via a parameter? > > > > > > > > > > > > > > > > > > IMO it would be nice if TC 3.0 made tenancy / roles & > > > > capabilities > > > > > a > > > > > > > > > requirement. No more turning it on and off. The scope of > what > > > you > > > > > see > > > > > > > is > > > > > > > > > dictated by your tenancy and the api's that you have > access to > > > > are > > > > > > > > dictated > > > > > > > > > by your capabilities. > > > > > > > > > > > > > > > > > > Jeremy > > > > > > > > > > > > > > > > > > On Thu, Jun 14, 2018 at 11:49 AM, Volz, Dylan < > > > > > > [email protected]> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Hi Traffic Controllers, > > > > > > > > > > > > > > > > > > > > I am working on enforcing the roles->capabilities system > as a > > > > > > > > replacement > > > > > > > > > > for the soon-to-be legacy priv level system. Like tenancy > > > this > > > > > is a > > > > > > > > > feature > > > > > > > > > > moving us towards self-service; so to minimize our > > > > code/behavior > > > > > > > paths > > > > > > > > > > I would like to propose renaming the use_tenancy > parameter to > > > > > > > > > > use_self_service, so that if it is turned on both > tenancy and > > > > > > > > > capabilities > > > > > > > > > > are applied. This prevents some hairy cases arising when > > > > > > capabilities > > > > > > > > are > > > > > > > > > > on and tenancy is off or vice versa. Let me know if you > have > > > > any > > > > > > > > > questions, > > > > > > > > > > concerns, or suggestions. > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > Dylan > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
