Re: [VOTE] Release Apache Traffic Control (incubating) 2.2.0-RC4

[Jeremy Mitchell]

+1

Tested:

TP
TO API

On Fri, Apr 13, 2018 at 3:46 PM, Jeff Elsloo  wrote:

> +1
>
> Tested:
>   -`./pkg`
>   - signatures validate
>   - upgrade of Traffic Router works without issue
> --
> Thanks,
> Jeff
>
>
> On Fri, Apr 13, 2018 at 10:26 AM, Dan Kirkwood  wrote:
> > +1
> >
> > I tested:
> > - signature and checksum good
> > - `./pkg -v` builds all components successfully
> > - traffic_ops installs on clean Centos 7 VM
> > - postinstall properly configures
> > - a couple of basic traffiic_ops calls return proper json output
> >
> >
> > On Mon, Apr 9, 2018 at 3:58 PM, David Neuman 
> > wrote:
> >
> >> +1
> >> I tested the following:
> >> - building with the pkg command
> >> - signatures
> >> - checksums (were we supposed to get rid of md5?  I must have missed
> that)
> >> - installed and started traffic_stats
> >>
> >>
> >> On Wed, Apr 4, 2018 at 11:04 AM, Robert Butts  wrote:
> >>
> >> > Hello All,
> >> >
> >> > I've prepared a release for v2.2.0-RC4
> >> >
> >> > The vote is open for at least 72 hours and passes if a majority of at
> >> least
> >> > 3 +1 PPMC votes are cast.
> >> >
> >> > [ ] +1 Approve the release
> >> >
> >> > [ ] -1 Do not release this package because ...
> >> >
> >> > Changes since 2.1.0:
> >> > https://github.com/apache/incubator-trafficcontrol/
> compare/RELEASE-2.1.0
> >> .
> >> > ..
> >> > RELEASE-2.2.0-RC4
> >> >
> >> > This corresponds to git:
> >> >  Hash: 45151f8e518fe92dfa64c7311b877cb13299debc
> >> >  Tag: RELEASE-2.2.0-RC4
> >> >
> >> > Which can be verified with the following: git tag -v RELEASE-2.2.0-RC4
> >> >
> >> > My code signing key is available here:
> >> > http://keys.gnupg.net/pks/lookup?search=0xFDD04F7F=vindex
> >> >
> >> > Make sure you refresh from a key server to get all relevant
> signatures.
> >> >
> >> > The source .tar.gz file, pgp signature (.asc signed with my key from
> >> > above), and sha512 checksum are provided here:
> >> >
> >> > https://dist.apache.org/repos/dist/dev/incubator/
> >> trafficcontrol/2.2.0/RC4
> >> >
> >> >
> >> >
> >> > Thanks!
> >> >
> >>
>

Re: Updated TO API guidelines

[Jeremy Mitchell]

@dave - you're right. the PR referenced above did not update version, docs
or tests.

re: version - these were very new endpoints. i simply rewrote it to follow
the new api guidelines. i think the risk of doing that was low as TP is the
only consumer of this endpoint.
re: docs - docs for 1.3 endpoints haven't been written. we're going to
depend on swagger for that. right dewayne?
re: tests - yep, i failed to update the existing tests and broke them and
submitted this subsequent PR -
https://github.com/apache/incubator-trafficcontrol/pull/2106

@rawlin -

i agree. i'd love to dump support for the `?(.json) ` suffix. seems
redundant. i would suggest that any new GET routes don't include it.

On Mon, Apr 9, 2018 at 12:02 PM, Rawlin Peters <rawlin.pet...@gmail.com>
wrote:

> Thanks for the example, Jeremy. Do we have any guidelines about
> whether or not the `?(.json) ` suffix should be supported for new API
> endpoints? It seems pointless because the API will always return JSON.
>
> - Rawlin
>
> On Fri, Apr 6, 2018 at 3:14 PM, Jeremy Mitchell <mitchell...@gmail.com>
> wrote:
> > Rawlin,
> >
> > I have submitted a PR to change some new ds request routes to utilize
> query
> > params instead of path/route params (the legacy format):
> >
> > https://github.com/apache/incubator-trafficcontrol/pull/2094
> >
> > On Fri, Apr 6, 2018 at 11:59 AM, Rawlin Peters <rawlin.pet...@gmail.com>
> > wrote:
> >
> >> Do we currently have an example of an API endpoint written in the
> >> traffic_ops_golang framework that uses only query parameters like
> >> this? How would it compare to the legacy format?
> >>
> >> -Rawlin
> >>
> >> On Thu, Apr 5, 2018 at 9:45 AM, Dewayne Richardson <dewr...@apache.org>
> >> wrote:
> >> > Thank you John for giving us "API Use Cases" to think about with these
> >> new
> >> > TO API Guidelines.  The main goal for these changes are to build TO
> API's
> >> > that are intuitive.  I'm of the opinion that if the API's are
> intuitive
> >> > (with easy to understand patterns) that prevents me from wasting time
> >> > looking up API docs.  A nice side effect of having simple standards
> and
> >> > patterns is that simplicity ripples into our API Go code which allows
> us
> >> to
> >> > easily build frameworks that do all of the work instead of the API
> >> > snowflakes that we have today.
> >> >
> >> > I encourage everyone to shoot as many holes into our thoughts around
> this
> >> > new direction so that we can see the bigger picture.
> >> >
> >> > -Dew
> >> >
> >> > On Wed, Apr 4, 2018 at 10:29 PM, John Rushford <jjrushf...@gmail.com>
> >> wrote:
> >> >
> >> >> Why the change?  It’s my understanding that path parameters should be
> >> used
> >> >> to specify a particular resource
> >> >> and query parameters should be used to sort/filter the query.  Why
> use a
> >> >> query parameter to specify a particular
> >> >> resource?  Is this REST API best practice?
> >> >>
> >> >> What about sub resource queries such as using the following:
> >> >>
> >> >> GET api/1.3/deliveryservices/{xmlID}/urisigning
> >> >>
> >> >> where you are requesting a particular urisigning keys sub resource
> for
> >> the
> >> >> particular deliveryservice resource. You can make it work
> >> >> with an xmlid query parameter but what do you return if the query
> >> >> parameter is left off, all uri signing keys?  Is that useful?
> >> >>
> >> >> John
> >> >>
> >> >> > On Apr 4, 2018, at 3:23 PM, Jeremy Mitchell <mitchell...@gmail.com
> >
> >> >> wrote:
> >> >> >
> >> >> > tbh i'm not sure about versioning. I was just trying to suggest
> that
> >> new
> >> >> > routes be formulated this way per the new API guidelines:
> >> >> >
> >> >> > GET /foos[?id, name, etc=]
> >> >> > POST /foos
> >> >> > PUT /foos [?id, name, etc=]
> >> >> > DELETE /foos [?id, name, etc=]
> >> >> >
> >> >> > instead of the old way:
> >> >> >
> >> >> > GET /foos
> >> >> > GET /foos/:id
> >> >> > POST /foos
> >> >> > PUT /foos/:id
> >> &

Re: Updated TO API guidelines

[Jeremy Mitchell]

John,

TBH, it's hard to find any hard and fast rules regarding REST. At least I
haven't found any.

So the question is why B instead of A:

A. GET api/1.3/deliveryservices/{xmlID}/urisigning
B. GET api/1.3/deliveryservices_urisigningkeys[?optional queryParams]

I can think of a few reasons:

1. query params are unambiguous. for example, they look like ?xmlId=foobar.
there is a key and a value. looking at
api/1.3/deliveryservices/foobar/urisigning,
it may not be clear that foobar is the xmlId of the delivery service.
2. pattern matching can get screwed up. think of this example. we have 2
routes defined:

GET /foos/:id
GET /foos/:name

which route does this match? GET /foos/42

maybe names can be numbers. so is 42 an ID or a Name? not sure.

3. we can get away with less route definitions. instead of 3 routes:

GET /foos <-- get all foos
GET /foos/:id <-- get foo by id
GET /foos/:name <-- get foo by name

we can simply have 1:

GET /foos with optional query params (?id= or ?name=)

I know there have been a lot of complaints about having to fetch resources
by id instead of name. This will take care of that.

4. Nested routes can start to get a little nasty and uneccessary imo. GET
/foos/:id/bars/:id/comments/:id (there can be a lot of permutations of
that) where you really only need the id of the comment to retrieve it: GET
/foo_bar_comments?id=4

5. There are some built in benefits in Go but I'll have to defer to Rob
and/or Dylan regarding that.

Off the top of my head, that's all i can think of. If we plan to use tools
such as Swagger for our API, we need to agree on a standard for the API and
stick to it. We're simply trying to find that standard. :)

Jeremy





On Wed, Apr 4, 2018 at 10:29 PM, John Rushford <jjrushf...@gmail.com> wrote:

> Why the change?  It’s my understanding that path parameters should be used
> to specify a particular resource
> and query parameters should be used to sort/filter the query.  Why use a
> query parameter to specify a particular
> resource?  Is this REST API best practice?
>
> What about sub resource queries such as using the following:
>
> GET api/1.3/deliveryservices/{xmlID}/urisigning
>
> where you are requesting a particular urisigning keys sub resource for the
> particular deliveryservice resource. You can make it work
> with an xmlid query parameter but what do you return if the query
> parameter is left off, all uri signing keys?  Is that useful?
>
> John
>
> > On Apr 4, 2018, at 3:23 PM, Jeremy Mitchell <mitchell...@gmail.com>
> wrote:
> >
> > tbh i'm not sure about versioning. I was just trying to suggest that new
> > routes be formulated this way per the new API guidelines:
> >
> > GET /foos[?id, name, etc=]
> > POST /foos
> > PUT /foos [?id, name, etc=]
> > DELETE /foos [?id, name, etc=]
> >
> > instead of the old way:
> >
> > GET /foos
> > GET /foos/:id
> > POST /foos
> > PUT /foos/:id
> > DELETE /foos/:id
> >
> > The difference being the use of query params over route/path params.
> >
> > Technically, adding new routes does not break old stuff right so i don't
> > think that warrants a major version roll.
> >
> > While we're on the subject, what does everyone think if we took this one
> > step further and made routes handle a request payload with one or more
> > items. For example:
> >
> > GET /foos[?id, name, etc=]
> > POST /foos <-- takes in an array of foos to create
> > PUT /foos <-- takes in an array of foos to update
> > DELETE /foos <-- takes in an array of foos to delete
> >
> > in this scenario, query params only pertain to the GET. The POST, PUT and
> > DELETE rely on the contents of the request json...
> >
> > Jeremy
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > On Wed, Apr 4, 2018 at 1:55 PM, Robert Butts <robert.o.bu...@gmail.com>
> > wrote:
> >
> >> That document doesn't mention versions, 1.2 vs 1.3 vs 2.0.
> >>
> >> Just to clarify, changing to query parameters breaks compatibility with
> 1.2
> >> and older, so new APIs in that format have to be a new major version,
> i.e.
> >> 2.0, per Semantic Versioning, right?
> >>
> >> On Tue, Apr 3, 2018 at 3:26 PM, Jeremy Mitchell <mitchell...@apache.org
> >
> >> wrote:
> >>
> >>> FYI - I've updated the TO API guidelines to reflect our desire to move
> >> away
> >>> from route/path params and embrace query params in the Golang API.
> >>>
> >>> https://cwiki.apache.org/confluence/display/TC/API+Guidelines
> >>>
> >>> Jeremy
> >>>
> >>
>
>

Re: Traffic Control Doc's Proposal

[Jeremy Mitchell]

Can you clarify what a "howto" is?

Yes, it looks like our README's are a bit out of control. What if we
consolidated them like such:

./README.md <-- this one links to general information about TC
(readthedocs) and also links to the following:

./traffic_portal/README.md
./traffic_monitor/README.md
./traffic_ops/README.md <-- this links to ./traffic_ops_db/README.md
./traffic_router/README.md
./traffic_server/README.md
./traffic_stats/README.md

^^ each of these readme's include 3 things at the very least:

- How to install
- How to develop
- A link to component info found in readthedocs (rst)

Jeremy

On Wed, Apr 4, 2018 at 9:48 AM, Dewayne Richardson 
wrote:

> I've been going through the Traffic Ops API doc's and revamping them with
> Swagger and noticed this trend with our documentation.   "Guides" (Howto's,
> Installs, Developer's)  growing because they are easier to maintain
> (because they are less fancy) as markdown files in the Github project.  My
> concern is the RST equivalents here:
> http://traffic-control-cdn.readthedocs.io/en/latest/development/index.html
> are going stale and they are also being duplicated in the markdown files.
> I also worry that the markdown files aren't surfaced properly to our
> community because there is valuable information in them as well.
>
> Therefore I propose the following documentation "rules of thumb":
>
> 1. Howto's, Installs, Developer/Admin Guides are done in Markdown but have
> RST links (or some reference to an index page to surface all of them)
> 2. High level informative documentation like API docs, and Traffic Control
> features are done in RST as we have been to surface that information for
> "Users of Traffic Control"
>
> Please provide your feedback,
>
> -Dew
>
>
> Here is a quick list of the Markdown files I've found in the project so
> far:
>
>
> ./misc/git/README.md
> ./CODE_OF_CONDUCT.md
> ./experimental/traffic_router_golang/README.md
> ./test/traffic_ops_cfg/Readme.md
> ./test/router/dnssec/Readme.md
> ./test/router/Readme.md
> ./CHANGELOG.md
> ./traffic_portal/v1/README.md
> ./traffic_portal/test/end_to_end/README.md
> ./traffic_portal/README.md
> ./traffic_portal/build/README.md
> ./traffic_monitor_java/README.md
> ./traffic_monitor/tools/testcaches/README.md
> ./traffic_monitor/README.md
> ./traffic_server/plugins/astats_over_http/README.md
> ./traffic_server/README.md
> ./README.md
> ./traffic_ops_db/pg-migration/README.md
> ./traffic_ops/experimental/goto/README.md
> ./traffic_ops/experimental/auth/README.md
> ./traffic_ops/experimental/server/README.md
> ./traffic_ops/experimental/webfront/README.md
> ./traffic_ops/INSTALL.md
> ./traffic_ops/README.md
> ./traffic_ops/testing/compare/README.md
> ./traffic_ops/testing/api/docker/README.md
> ./traffic_ops/testing/api/README.md
> ./traffic_ops/build/README.md
> ./traffic_ops/client_tests/README.md
> ./traffic_ops/traffic_ops_golang/README.md
> ./traffic_ops/traffic_ops_golang/swaggerdocs/v13/README.md
> ./CONTRIBUTING.md
> ./BUILD.md
> ./build/README.md
> ./infrastructure/docker/README.md
> ./infrastructure/docker/build/README.md
> ./infrastructure/test/integration/README.md
> ./infrastructure/test/README.md
> ./traffic_router/core/src/test/resources/Readme.md
> ./traffic_router/neustar/README.md
>

Re: Updated TO API guidelines

[Jeremy Mitchell]

tbh i'm not sure about versioning. I was just trying to suggest that new
routes be formulated this way per the new API guidelines:

GET /foos[?id, name, etc=]
POST /foos
PUT /foos [?id, name, etc=]
DELETE /foos [?id, name, etc=]

instead of the old way:

GET /foos
GET /foos/:id
POST /foos
PUT /foos/:id
DELETE /foos/:id

The difference being the use of query params over route/path params.

Technically, adding new routes does not break old stuff right so i don't
think that warrants a major version roll.

While we're on the subject, what does everyone think if we took this one
step further and made routes handle a request payload with one or more
items. For example:

GET /foos[?id, name, etc=]
POST /foos <-- takes in an array of foos to create
PUT /foos <-- takes in an array of foos to update
DELETE /foos <-- takes in an array of foos to delete

in this scenario, query params only pertain to the GET. The POST, PUT and
DELETE rely on the contents of the request json...

Jeremy











On Wed, Apr 4, 2018 at 1:55 PM, Robert Butts <robert.o.bu...@gmail.com>
wrote:

> That document doesn't mention versions, 1.2 vs 1.3 vs 2.0.
>
> Just to clarify, changing to query parameters breaks compatibility with 1.2
> and older, so new APIs in that format have to be a new major version, i.e.
> 2.0, per Semantic Versioning, right?
>
> On Tue, Apr 3, 2018 at 3:26 PM, Jeremy Mitchell <mitchell...@apache.org>
> wrote:
>
> > FYI - I've updated the TO API guidelines to reflect our desire to move
> away
> > from route/path params and embrace query params in the Golang API.
> >
> > https://cwiki.apache.org/confluence/display/TC/API+Guidelines
> >
> > Jeremy
> >
>

Re: [VOTE] Resolution for Traffic Control graduation to TLP

[Jeremy Mitchell]

+1

Jeremy

On Mon, Apr 2, 2018 at 10:28 PM, Jason Tucker <jasonwtuc...@gmail.com>
wrote:

> +1
>
> On Tue, Apr 3, 2018 at 2:58 AM, John Rushford <jjrushf...@gmail.com>
> wrote:
>
> > +1
> >
> > > On Apr 2, 2018, at 7:07 PM, Eric Friedrich (efriedri) <
> > efrie...@cisco.com> wrote:
> > >
> > > +1
> > >> On Apr 2, 2018, at 4:11 PM, David Neuman <david.neuma...@gmail.com>
> > wrote:
> > >>
> > >> Dear Traffic Control community members:
> > >>
> > >> I would like to call a vote on the resolution for Traffic Control to
> > >> graduate from to an Apache TLP.  We have already voted on whether or
> > not we
> > >> should start the graduation process [1] and this is the next step.
> > Please
> > >> see the resolution below and vote as follows:
> > >>
> > >> [ ] +1 Graduate Traffic Control from the incubator
> > >> [ ] +0 No Opinion
> > >> [ ] -1 Do not graduate Traffic Control from the incubator (please
> > provide a
> > >> reason)
> > >>
> > >> The vote is open for a minimum of 72 hours and will need at least 3
> > more +1
> > >> votes than -1 votes from PMC members to succeed.
> > >>
> > >> If this VOTE succeeds, a similar VOTE will be started on the
> > >> gene...@incubator.apache.org mailing list. If that VOTE succeeds, a
> > >> resolution will be included in the next Apache Board Meeting.
> > >>
> > >> Please feel free to reach out with any questions.
> > >>
> > >> Thanks,
> > >> Dave
> > >>
> > >> [1]
> > >> https://lists.apache.org/thread.html/fb1fae0785feb6568cef6deb6fa207
> > 23eba54ed63a445462d44564d3@%3Cdev.trafficcontrol.apache.org%3E
> > >>
> > >> -
> > >>
> > >> Establish the Apache Traffic Control Project
> > >>
> > >> WHEREAS, the Board of Directors deems it to be in the best interests
> of
> > >> the Foundation and consistent with the Foundation's purpose to
> establish
> > >> a Project Management Committee charged with the creation and
> maintenance
> > >> of open-source software, for distribution at no charge to the public,
> > >> related to building, monitoring, configuring, and provisioning a large
> > >> scale content delivery network (CDN)..
> > >>
> > >> NOW, THEREFORE, BE IT RESOLVED, that a Project Management Committee
> > >> (PMC), to be known as the "Apache Traffic Control Project", be and
> > >> hereby is established pursuant to Bylaws of the Foundation; and be it
> > >> further
> > >>
> > >> RESOLVED, that the Apache Traffic Control Project be and hereby is
> > >> responsible for the creation and maintenance of software related to
> > >> building, monitoring, configuring, and provisioning a large scale
> > >> content delivery network (CDN).; and be it further
> > >>
> > >> RESOLVED, that the office of "Vice President, Apache Traffic Control"
> be
> > >> and hereby is created, the person holding such office to serve at the
> > >> direction of the Board of Directors as the chair of the Apache Traffic
> > >> Control Project, and to have primary responsibility for management of
> > >> the projects within the scope of responsibility of the Apache Traffic
> > >> Control Project; and be it further
> > >>
> > >> RESOLVED, that the persons listed immediately below be and hereby are
> > >> appointed to serve as the initial members of the Apache Traffic
> Control
> > >> Project:
> > >>
> > >> * Dan Kirkwood   <dang...@apache.org>
> > >> * David Neuman   <neu...@apache.org>
> > >> * Dewayne Richardson <dewr...@apache.org>
> > >> * Eric Covener   <cove...@apache.org>
> > >> * Eric Friedrich <fri...@apache.org>
> > >> * Hank Beatty<hbea...@apache.org>
> > >> * Jan van Doorn  <j...@apache.org>
> > >> * Jeff Elsloo<els...@apache.org>
> > >> * Jeremy Mitchell<mitchell...@apache.org>
> > >> * Leif Hedstrom  <zw...@apache.org>
> > >> * Mark Torluemke <mtorlue...@apache.org>
> > >> * Phil Sorber<sor...@apache.org>
> > >> * Steve Malenfant<smalenf...@apache.org>
> > >>
> > >> NOW, THEREFORE, BE IT FURTHER RESOLVED, that David Neuman be appointed
> > >> to the office of Vice President, Apache Traffic Control, to serve in
> > >> accordance with and subject to the direction of the Board of Directors
> > >> and the Bylaws of the Foundation until death, resignation, retirement,
> > >> removal or disqualification, or until a successor is appointed; and be
> > >> it further
> > >>
> > >> RESOLVED, that the initial Apache Traffic Control PMC be and hereby is
> > >> tasked with the creation of a set of bylaws intended to encourage open
> > >> development and increased participation in the Apache Traffic Control
> > >> Project; and be it further
> > >>
> > >> RESOLVED, that the Apache Traffic Control Project be and hereby is
> > >> tasked with the migration and rationalization of the Apache Incubator
> > >> Traffic Control podling; and be it further
> > >>
> > >> RESOLVED, that all responsibilities pertaining to the Apache Incubator
> > >> Traffic Control podling encumbered upon the Apache Incubator PMC are
> > >> hereafter discharged.
> > >
> >
> >
>

Updated TO API guidelines

[Jeremy Mitchell]

FYI - I've updated the TO API guidelines to reflect our desire to move away
from route/path params and embrace query params in the Golang API.

https://cwiki.apache.org/confluence/display/TC/API+Guidelines

Jeremy

Re: Traffic Ops Change log

[Jeremy Mitchell]

Can't argue with that. Our change log needs to be more robust imo. Like you
said, what exactly changed on server with id=42?

On Fri, Mar 23, 2018 at 9:08 AM, Chris Lemmons <alfic...@gmail.com> wrote:

> The changelog exists to answer the question, "Who made things this way
> and when?"
>
> I think we do need changelog entries on everything change that changes
> things, but those comments aren't useful unless they tell us what
> changed and what the old and new values are. So, I'm not in favour of
> filling the changelog with a whole bunch of:
>
> myusername - Updated server ['Foo'] with id: 42
>
> But I'd be quite interested in this:
>
> myusername - Updated server ['Foo'] with id 42: Changed 'origin' from
> 'http://foo.example.com' to 'http://bar.example.com'.
>
> I think that moves it from being spammy to being useful. Might take a
> bit of thought to make sure we don't create epic changelog messages
> when someone creates a whole new object or changes a large number of
> things, but overall I think it would be very useful.
>
> WRT the DSRs, I think that considering them to be Change Logs
> themselves might work at some point, but it's not quite there yet. I
> would include the changes to DSRs.
>
> On Wed, Mar 21, 2018 at 10:46 AM, Dewayne Richardson <dewr...@apache.org>
> wrote:
> > IMO, I don't think it's necessary to pollute the Change Log with Delivery
> > Service Requests (DSRs) comments.  I think DSRs are turning out to be the
> > Change Log for Delivery Services and comments on those are Change Log
> noise.
> >
> > -Dew
> >
> > On Wed, Mar 21, 2018 at 10:24 AM, Jeremy Mitchell <
> mitchell...@apache.org>
> > wrote:
> >
> >> Currently, every POST (create), PUT (update) or DELETE (delete) TO API
> >> endpoint (or most of them) create a change log entry.
> >>
> >> For example when i PUT /api/cachegroups/4 it creates this CL entry:
> >>
> >> myusername - Updated Cachegroup named 'Foo' with id: 57
> >>
> >> Question: In the golang rewrite of the TO API should EVERY POST, PUT
> >> and DELETE trigger a change log entry? Or should it be optional and
> >> determine by the developer?
> >>
> >> I ask because I'm adding endpoints for create, update and delete of
> >> delivery service request comments and it feels like too much to create a
> >> change log entry for that. I'd hate to pollute the change log for minor
> >> stuff.
> >>
> >> To me, it seems like it should be optional and determined by the API
> >> developer.
> >>
> >> Jeremy
> >>
>

Traffic Ops Change log

[Jeremy Mitchell]

Currently, every POST (create), PUT (update) or DELETE (delete) TO API
endpoint (or most of them) create a change log entry.

For example when i PUT /api/cachegroups/4 it creates this CL entry:

myusername - Updated Cachegroup named 'Foo' with id: 57

Question: In the golang rewrite of the TO API should EVERY POST, PUT
and DELETE trigger a change log entry? Or should it be optional and
determine by the developer?

I ask because I'm adding endpoints for create, update and delete of
delivery service request comments and it feels like too much to create a
change log entry for that. I'd hate to pollute the change log for minor
stuff.

To me, it seems like it should be optional and determined by the API
developer.

Jeremy

Re: Traffic Ops Access Control v2

[Jeremy Mitchell]

Dave,

1. Yes, the "proposed" roles are default roles that will be seeded with TO.
The idea is that you can use those roles or create your own.

2. Really, there should be no need to CRUD capabilities. for example, the
ds-read capability should always include the GET /api/deliveryservices and
GET /api/deliveryservices/:id routes. However, we do have the concept of
API extensions. For example, let's say Comcast has some extra routes (GET
/api/foos and GET /api/foos/:id). In that case, Comcast would need to
create a foos-read capability. So maybe CRUDing capabilities makes sense.
Not sure yet TBH.

3. Yes, the Admin role will be seeded to include ALL capabilities. The Ops
role will be seeded with whatever capabilities we deem appropriate. There
will probably need to be some discussion here about which capabilities the
Ops role gets. Actually, there will have to be discussion about which
capabilities each role gets. I will probably make a proposal and then get
feedback.

Jeremy

On Mon, Mar 12, 2018 at 1:02 PM, Dave Neuman <neu...@apache.org> wrote:

> This sounds great Jeremy, looking forward to it getting implemented.  A few
> things though:
>
> 1) The "proposed roles" are really just "default roles" right?  Meaning we
> will provide a way to create new roles and assign capabilities to them?
> 2) We will provide a way to CRUD capabilities, correct?
> 3) Is it assumed that Admin gets everything?  What does Operations NOT get
> that admin DOES get?  Trying to differentiate between the two.
>
> Thanks,
> Dave
>
> On Thu, Mar 8, 2018 at 9:53 AM, Jeremy Mitchell <mitchell...@apache.org>
> wrote:
>
> > There has been some discussion for quite some time regarding an overhaul
> of
> > the TO access control model. I'd like to refresh eveyone's memory on that
> > discussion.
> >
> >
> > *Current system:*
> >
> > Since the beginning, resources (or routes (UI and API)) have been locked
> > down by role, or more specifically, privilege level. For example, if a
> > route requires a privilege level of 20, then only users with the
> operations
> > role (priv level=20) or admin role (priv level=30) could access that
> route.
> > Here are our current roles (and their priv levels):
> >
> > Admin (30)
> > Operations (20)
> > Portal (15)
> > Federation (15)
> > Steering (15)
> > ORT (11)
> > Read-Only (10)
> > Disallowed (0)
> >
> > This method has served us well for quite a while but there are some
> > drawbacks to this approach. Here are a few I can think of:
> >
> > - No clear understanding of which routes each role provides access to.
> For
> > example, what is the difference between the Admin and Operations role?
> All
> > I know is that the admin role has a priv level of 30 and operations has
> 20.
> > I can't tell you which routes an admin has access to that operations does
> > not without reading the code or going thru all the docs. Ain't nobody got
> > time for that!
> >
> > - The "Additive" nature of the roles (via priv level) prevents the
> ability
> > to create unrelated roles. You can't create 2 roles with unique access.
> > Higher level roles always inherit from lower level roles. The Federation
> > role is a good example. Federation users only need access to a couple
> > routes yet since it has a priv-level=15, federation users look like they
> > can do federation, steering, portal, ort and read-only things...
> >
> > - Not easy to alter the access level of a role. For example, if you
> wanted
> > the Portal role to have access to a few more routes, what would you do?
> > Raise priv level to 18? Not sure what that would do...if anything. You'd
> > have to make code changes to ensure an 18 would actually do something.
> >
> > - Many API consumers have elevated permissions when they only need access
> > to a few routes. I.e. traffic monitors, traffic routers, traffic stats
> all
> > have to be given the admin role. so basically, they've been granted
> access
> > to do EVERYTHING when they only access a few routes.
> >
> > - There is also inconsistency in how roles are enforced. Most routes use
> > priv level to determine access but some routes simply check if the user
> has
> > the role (i.e. steering).
> >
> >
> > *New proposed system:*
> >
> > *Tenancy*
> >
> > Last summer tenancy was introduced (thanks Qwilt) giving us the ability
> to
> > "scope" certain resources (delivery services, users and also tenants) to
> > certain users. This was a big step towards self-service as we can now
> limit
> > w

Traffic Ops Access Control v2

[Jeremy Mitchell]

There has been some discussion for quite some time regarding an overhaul of
the TO access control model. I'd like to refresh eveyone's memory on that
discussion.


*Current system:*

Since the beginning, resources (or routes (UI and API)) have been locked
down by role, or more specifically, privilege level. For example, if a
route requires a privilege level of 20, then only users with the operations
role (priv level=20) or admin role (priv level=30) could access that route.
Here are our current roles (and their priv levels):

Admin (30)
Operations (20)
Portal (15)
Federation (15)
Steering (15)
ORT (11)
Read-Only (10)
Disallowed (0)

This method has served us well for quite a while but there are some
drawbacks to this approach. Here are a few I can think of:

- No clear understanding of which routes each role provides access to. For
example, what is the difference between the Admin and Operations role? All
I know is that the admin role has a priv level of 30 and operations has 20.
I can't tell you which routes an admin has access to that operations does
not without reading the code or going thru all the docs. Ain't nobody got
time for that!

- The "Additive" nature of the roles (via priv level) prevents the ability
to create unrelated roles. You can't create 2 roles with unique access.
Higher level roles always inherit from lower level roles. The Federation
role is a good example. Federation users only need access to a couple
routes yet since it has a priv-level=15, federation users look like they
can do federation, steering, portal, ort and read-only things...

- Not easy to alter the access level of a role. For example, if you wanted
the Portal role to have access to a few more routes, what would you do?
Raise priv level to 18? Not sure what that would do...if anything. You'd
have to make code changes to ensure an 18 would actually do something.

- Many API consumers have elevated permissions when they only need access
to a few routes. I.e. traffic monitors, traffic routers, traffic stats all
have to be given the admin role. so basically, they've been granted access
to do EVERYTHING when they only access a few routes.

- There is also inconsistency in how roles are enforced. Most routes use
priv level to determine access but some routes simply check if the user has
the role (i.e. steering).


*New proposed system:*

*Tenancy*

Last summer tenancy was introduced (thanks Qwilt) giving us the ability to
"scope" certain resources (delivery services, users and also tenants) to
certain users. This was a big step towards self-service as we can now limit
what certain users see. Access control is now role + tenancy (if tenancy is
applicable and turned on via the use_tenancy parameter).

*Roles/Capabilities*

Actually, a lot of work has already been done (thanks again, Qwilt) in this
area but of course, there is more to do. Let me explain a bit how it works
conceptually.

Proposed Roles:

Admin
Operations
Content Provider (formerly known as Portal)
Federation
Cache (formerly known as ORT)
Monitor (new)
Router (new)
Stats (new)
Read-Only
Disallowed

Concept:

- a user has one role
- a role has N capabilities (i.e. ds-read, ds-write, etc)
- a capability is mapped to N API endpoints (i.e. ds-read is mapped to GET
/api/deliveryservices and GET /api/deliveryservices/:id)

A user's capabilities (and not privilege level) determine whether a user
has access to an API endpoint or not.

Advantages:

- By mapping roles to capabilities and capabilities to API endpoints, it's
easy to see what level of API access each role provides. For example, easy
to see the difference between the Admin and Operations role.

- Roles are not "additive". Unrelated, unique roles can be created. For
example, the Federation role and Content Provider role (formerly Portal
role) can provide 2 completely different levels of access control.
Currently, they provide the exact same level of access because of priv
level.

- Tightly defined roles with specific capabilities provides better
security. I.e. you don't have to give a user an admin role so they can do
only 2 things.

- Can create custom roles on the fly to only include access to certain API
endpoints. If you want to create a Bob role with just the ds-read
capability, go for it. You can get very creative with your roles if you
want to. Or you can just use those that are provided.

Disadvantages:

- More setup required. All API endpoints need to be grouped into
capabilities (again, Qwilt did a lot of work in this area). Capabilities
need to be added to the appropriate roles.

If you haven't read enough at this point and are thirsty for more. There is
more here:
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=68715910

Thanks for reading. Looking forward to your comments/concerns.

Jeremy

Traffic Ops UI Deprecation in favor of Traffic Portal

[Jeremy Mitchell]

At last year's TC summit, we discussed the deprecation of the TO UI in
favor of the Traffic Portal.

Now that TP has almost achieved feature parity with the TO UI, that day is
coming soon.
Here are 2 issues I know that need to be complete to achieve feature parity:

https://github.com/apache/incubator-trafficcontrol/issues/1616
https://github.com/apache/incubator-trafficcontrol/issues/1287

Deprecation may mean simply deleting traffic_ops/app/public/ for 2.3 (or
3.x). Or maybe it means something else. Not sure yet.

But the point is, if you want to ensure that all new UI features that you
build live past 2.3 (or 3.x), please be sure to put those features in TP.
Reach out to me if you need help with TP.

Jeremy

Re: [VOTE] Traffic Control graduation to TLP

[Jeremy Mitchell]

+1

On Thu, Mar 1, 2018 at 2:06 PM, Nir Sopher  wrote:

> +1
>
> On Mar 1, 2018 10:27 PM, "Steve Malenfant"  wrote:
>
> > +1
> >
> > On Thu, Mar 1, 2018 at 12:14 PM, Phil Sorber  wrote:
> >
> > > +1
> > >
> > > On Thu, Mar 1, 2018 at 1:12 PM Hank Beatty  wrote:
> > >
> > > > +1
> > > >
> > > > On 03/01/2018 10:41 AM, Dave Neuman wrote:
> > > > >   Hey All,
> > > > >
> > > > > After a great discussion amongst the Apache Traffic Control PPMC,
> > > > reviewing
> > > > > the graduation checklist[1], updating the podling status page[2],
> and
> > > > > updating the project website to ensure the whimsy podling website
> > > checks
> > > > > pass[3], I would like to call a vote for Apache Traffic Control
> > > > graduating
> > > > > to a top level project.
> > > > >
> > > > > Apache Traffic Control entered the incubator on July 12, 2016.
> Since
> > > > then
> > > > > we have announced 4 releases, nominated 4 new committers,
> organized 3
> > > > > summits, had almost 8,000 commits from 63 different contributors,
> and
> > > --
> > > > > most importantly -- we have grown and diversified our community.
> > > Apache
> > > > > Traffic Control is a healthy project that is already acting like an
> > > > Apache
> > > > > top level project, so we should take the next step.
> > > > >
> > > > > If we agree that we should graduate to a top level project, the
> next
> > > > steps
> > > > > will be to pick the initial PMC chair for the TLP and draft a
> > > Resolution
> > > > > for the PPMC and IPMC to vote upon.
> > > > >
> > > > > Please take a minute to vote on wheter or not Traffic Control
> should
> > > > > graduate to a Top Level Project by responding with one of the
> > > following:
> > > > >
> > > > > [ ] +1 Apache Traffic Control should graduate.
> > > > > [ ] +0 No opinion
> > > > > [ ] -1 Apache Traffic Control should not graduate (please provide
> the
> > > > > reason)
> > > > >
> > > > > The VOTE will be opened for at least the next 72 hours.  Per Apache
> > > > > guidelines[4] I will also be notifying the incubator mailing list
> > that
> > > a
> > > > > community vote is under way.
> > > > >
> > > > > Thanks,
> > > > > Dave
> > > > >
> > > > >
> > > > > [1]
> > > > >
> > > > https://incubator.apache.org/guides/graduation.html#
> > > graduation_check_list
> > > > > [2] http://incubator.apache.org/projects/trafficcontrol.html
> > > > > [3] https://whimsy.apache.org/pods/project/trafficcontrol
> > > > > [4]
> > > > >
> > > > https://incubator.apache.org/guides/graduation.html#
> > > community_graduation_vote
> > > > >
> > > >
> > >
> >
>

Re: Github PR/Issues Format Templates

[Jeremy Mitchell]

Chris,

Unfortunately, some PRs contain many commits and even with good commit
messages, it's may be hard to figure out what exactly all the commits are
trying to accomplish. Therefore, I still think these 2 questions help any
committer/merger/reviewer get a quick handle on the intent of the PR:

At a high-level, what does this PR do?

 >> It creates unicorns when you click on the unicorn button.

What is the best way to verify this PR?

>> Open TP, click on the unicorn button in the top right corner, and watch
the pretty unicorns.

I can't tell you how many times I've had to read through commit messages
and/or code to try to figure out what a PR is trying to accomplish and how
to verify it. I'm just trying to save the reviewer/merger some time..

Jeremy













On Wed, Feb 28, 2018 at 1:31 PM, Chris Lemmons <alfic...@gmail.com> wrote:

> `What effect should I expect to see from this change?`
>
> This is a perfect question, and one that absolutely needs to be
> answered. But either
>
> a) it is answered already in the commit message; or
> b) the commit message is insufficient and needs to be `git -amend`ed.
>
> I definitely wouldn't want a PR that contained that information only
> in the PR body,
> and there's not a whole lot of value in asking them to re-type it. The
> "copy/paste" thing
> at the top is already a bit of duplication anyway.
>
> On Wed, Feb 28, 2018 at 11:03 AM, Dan Kirkwood <dang...@gmail.com> wrote:
> > followup: rather than
> >`What is the best way to verify this PR? `
> >
> > what about
> >
> > `What effect should I expect to see from this change?`
> >
> > On Wed, Feb 28, 2018 at 10:25 AM, Dan Kirkwood <dang...@gmail.com>
> wrote:
> >
> >> +1 on keeping it short and to the point...
> >>
> >> On Wed, Feb 28, 2018 at 10:14 AM, Jeremy Mitchell <
> mitchell...@gmail.com>
> >> wrote:
> >>
> >>> Chris, I really wanted the PR template to be less daunting and super
> short
> >>> and to the point. It's intention is to give a super quick summary of
> >>> what's
> >>> included in this PR to help the merger...
> >>>
> >>> Example:
> >>>
> >>>  What does this PR do? Is there a related Github issue?
> >>>
> >>> "See Issue #1245" or "This PR cascade deletes all delivery service
> regexes
> >>> when a delivery service is deleted"
> >>>
> >>>  What is the best way to verify this PR? <-- IMO this is really
> >>> important for the merger so I know how to "test" or "verify" the
> >>> functionality.
> >>>
> >>> Hit the DELETE /api/1.3/deliveryservices/:id endpoint and ensure all
> >>> entries in the deliveryservice_regex table are deleted for that
> delivery
> >>> service.
> >>>
> >>>  Does your PR include any of the following?
> >>>
> >>> - [ ] Tests
> >>> - [ ] Documentation
> >>> - [X] CHANGELOG.md entry
> >>>
> >>> ^^ I wasn't trying to imply that those last things were required. I
> just
> >>> wanted to provide a checklist that might be helpful for the contributer
> >>> and
> >>> the merger. For example, I always for get to look for a CHANGELOG.md
> >>> entry...
> >>>
> >>> Jeremy
> >>>
> >>>
> >>> On Tue, Feb 27, 2018 at 9:47 PM, Chris Lemmons <alfic...@gmail.com>
> >>> wrote:
> >>>
> >>> > If there's a relevant GitHub issue, that should be noted in the
> >>> > check-in comment, I think. Same for "what does it do?" I don't
> usually
> >>> > want to spell out steps for someone to verify my stuff because those
> >>> > are the steps that I took to verify it. The PR is so you can see the
> >>> > things I didn't see. And the commit list will make the presence of
> >>> > tests, documentation, and a changelog entry really obvious.
> >>> >
> >>> > Taking yours and reformatting a bit, what if we did something like
> this?
> >>> >
> >>> > ...
> >>> >
> >>> > *Describe your PR:* _(copy/paste from changeset comments is
> >>> encouraged!)_
> >>> >
> >>> >
> >>> >
> >>> > *Quick Checklist:*
> >>> >
> >>> > - [ ] Each commit message tells you everything you need to know about
>

Re: Github PR/Issues Format Templates

[Jeremy Mitchell]

Chris, I really wanted the PR template to be less daunting and super short
and to the point. It's intention is to give a super quick summary of what's
included in this PR to help the merger...

Example:

 What does this PR do? Is there a related Github issue?

"See Issue #1245" or "This PR cascade deletes all delivery service regexes
when a delivery service is deleted"

 What is the best way to verify this PR? <-- IMO this is really
important for the merger so I know how to "test" or "verify" the
functionality.

Hit the DELETE /api/1.3/deliveryservices/:id endpoint and ensure all
entries in the deliveryservice_regex table are deleted for that delivery
service.

 Does your PR include any of the following?

- [ ] Tests
- [ ] Documentation
- [X] CHANGELOG.md entry

^^ I wasn't trying to imply that those last things were required. I just
wanted to provide a checklist that might be helpful for the contributer and
the merger. For example, I always for get to look for a CHANGELOG.md
entry...

Jeremy


On Tue, Feb 27, 2018 at 9:47 PM, Chris Lemmons <alfic...@gmail.com> wrote:

> If there's a relevant GitHub issue, that should be noted in the
> check-in comment, I think. Same for "what does it do?" I don't usually
> want to spell out steps for someone to verify my stuff because those
> are the steps that I took to verify it. The PR is so you can see the
> things I didn't see. And the commit list will make the presence of
> tests, documentation, and a changelog entry really obvious.
>
> Taking yours and reformatting a bit, what if we did something like this?
>
> ...
>
> *Describe your PR:* _(copy/paste from changeset comments is encouraged!)_
>
>
>
> *Quick Checklist:*
>
> - [ ] Each commit message tells you everything you need to know about
> the change. (Squashing can help with this.)
> - [ ] If applicable, the commit message mentions the appropriate issue
> number.
> - [ ] This PR does *not* fix a serious security flaw. (Read more:
> www.apache.org/security )
> - [ ] Automatic code formatters (like gofmt) have been run.
>
> *Tests:*
>
> - [ ] Are not necessary.
> - [ ] Would be helpful, but aren't in this PR.
> - [ ] Are present, but incomplete.
> - [ ] Are included.
>
> *Doc updates:*
>
> - [ ] Are not necessary.
> - [ ] Would be helpful, but aren't in this PR.
> - [ ] Are present, but incomplete.
> - [ ] Are included.
>
> *If there's no update to CHANGELOG.md, why not?*
>
> *Does this break backward compatibility?*
>
> *Is there anyone specific that ought to take a look at this?*
>
> ...
>
> We want to be friendly to committers, while still getting good
> information for checking PRs. I could be easily convinced that the
> "Tests" or "Doc updates" sections in there are too long, but I think
> it should be clear that a potential committer can offer up some code
> without hitting 100% on tests, docs, and such.
>
> On Tue, Feb 27, 2018 at 1:24 PM, Jeremy Mitchell <mitchell...@gmail.com>
> wrote:
> > How about something like this for a PR template?
> >
> >  What does this PR do? Is there a relevant Github issue?
> >
> >
> >  What is the best way to verify this PR?
> >
> >
> >  Does your PR include any of the following?
> >
> > - [ ] Tests
> > - [ ] Documentation
> > - [ ] CHANGELOG.md entry
> >
> > On Tue, Feb 27, 2018 at 10:46 AM, Jeremy Mitchell <mitchell...@gmail.com
> >
> > wrote:
> >
> >> With an issue and/or pr template, we will have 6/6 items checked:
> >>
> >> https://github.com/apache/incubator-trafficcontrol/community
> >>
> >> I actually think PR templates would be quite helpful. As a
> >> committer/merger, it would be nice to know what problem the PR is
> solving
> >> and how to verify the functionality. A PR template could also help
> >> contributors ensure that their PRs are complete. I.e. does this PR
> includes
> >> tests, documentation, etc.
> >>
> >> I'll take a stab at a couple of templates and run them by the group.
> >>
> >> Jeremy
> >>
> >> On Wed, Jan 31, 2018 at 1:10 PM, Chris Lemmons <alfic...@gmail.com>
> wrote:
> >>
> >>> I'm +1 on Issue Templates, for sure. I don't know that PR templates
> >>> are quite as critical, but it might be nice to have a reminder in
> >>> there about verifying that the changelog is updated if necessary and
> >>> documentation for new features is present. If the PR Template
> >>> overwrites the default comment that you get from the commit body, it
> >>> m

Re: Github PR/Issues Format Templates

[Jeremy Mitchell]

How about something like this for a PR template?

 What does this PR do? Is there a relevant Github issue?


 What is the best way to verify this PR?


 Does your PR include any of the following?

- [ ] Tests
- [ ] Documentation
- [ ] CHANGELOG.md entry

On Tue, Feb 27, 2018 at 10:46 AM, Jeremy Mitchell <mitchell...@gmail.com>
wrote:

> With an issue and/or pr template, we will have 6/6 items checked:
>
> https://github.com/apache/incubator-trafficcontrol/community
>
> I actually think PR templates would be quite helpful. As a
> committer/merger, it would be nice to know what problem the PR is solving
> and how to verify the functionality. A PR template could also help
> contributors ensure that their PRs are complete. I.e. does this PR includes
> tests, documentation, etc.
>
> I'll take a stab at a couple of templates and run them by the group.
>
> Jeremy
>
> On Wed, Jan 31, 2018 at 1:10 PM, Chris Lemmons <alfic...@gmail.com> wrote:
>
>> I'm +1 on Issue Templates, for sure. I don't know that PR templates
>> are quite as critical, but it might be nice to have a reminder in
>> there about verifying that the changelog is updated if necessary and
>> documentation for new features is present. If the PR Template
>> overwrites the default comment that you get from the commit body, it
>> might be more annoying than valuable, though.
>>
>> I'm also +1 on hiding these particular files in a .github directory.
>> Unlike CONTRIBUTING and README, they don't provide all that much
>> benefit for a new person looking for stuff to read.
>>
>> On Wed, Jan 31, 2018 at 12:17 PM, Durfey, Ryan <ryan_dur...@comcast.com>
>> wrote:
>> > Always +1 on standardization and consistency
>> >
>> > I still want to circle back and setup project/kanbans for organizing
>> tickets in Github.
>> >
>> > Ryan DurfeyM | 303-524-5099
>> > CDN Support (24x7): 866-405-2993 or cdn_supp...@comcast.com> cdn_supp...@comcast.com>
>> >
>> > From: Dewayne Richardson <dewr...@gmail.com>
>> > Reply-To: "dev@trafficcontrol.incubator.apache.org" <
>> dev@trafficcontrol.incubator.apache.org>
>> > Date: Wednesday, January 31, 2018 at 11:15 AM
>> > To: "dev@trafficcontrol.incubator.apache.org" <
>> dev@trafficcontrol.incubator.apache.org>
>> > Subject: Github PR/Issues Format Templates
>> >
>> > I was working through the go-swagger repo and found a bug.  I submitted
>> a
>> > new issue and found this interesting approach I think the TC github
>> should
>> > adopt, "Issue and PR Templates".  I think the main value here is
>> > consistency in our PRs/Issues and user friendly prompts to say "these
>> are
>> > the data points we need to help you solve your issue".
>> >
>> > Working example:
>> > https://github.com/go-swagger/go-swagger/issues/new
>> >
>> > Github Doc on how to implement templates:
>> > https://github.com/blog/2111-issue-and-pull-request-templates
>> >
>> > If we think it's a good idea, then I'll respond with some examples for
>> > Issues and PR's that we can discuss.
>> >
>> > -Dew
>> >
>>
>
>

Re: Steering Target Geo-Ordering

[Jeremy Mitchell]

Makes perfect sense. thanks for the clarification.

On Tue, Feb 27, 2018 at 11:32 AM, Rawlin Peters <rawlin.pet...@gmail.com>
wrote:

> Hey Jeremy,
>
> With a redundant origin in each location, there would be a total of 6
> HTTP_LIVE DSes. Then you'd create a steering DS with each of those 6
> HTTP_LIVE DSes as targets. The type of each target would either be
> GEO_ORDER or GEO_WEIGHT, depending on if we want a forced ordering or
> something like a 50/50 split, respectively.
>
> Say I have the following targets in my Steering DS:
>
> Seattle1 (S1):
> GEO_ORDER: 1
>
> Seattle2 (S2):
> GEO_ORDER: 2
>
> Denver1 (D1):
> GEO_WEIGHT: 50
>
> Denver2 (D2):
> GEO_WEIGHT: 50
>
> Boston1 (B1):
> GEO_ORDER: 1
>
> Boston2 (B2):
> GEO_ORDER: 2
>
> For a client in Seattle requesting /foo, they would get the list [S1,
> S2, D2, D1, B1, B2], where D1 and D2 are consistent hashed based upon
> the requested URL and given weight. So if they request /bar maybe D1
> and D2 are switched so they get [S1, S2, D1, D2, B1, B2].
>
> - Rawlin
>
> On Mon, Feb 26, 2018 at 8:18 PM, Jeremy Mitchell <mitchell...@gmail.com>
> wrote:
> > So, back to the Seattle, Denver, Boston example...
> >
> > To accomplish the geo ordering and redundant origin use case, you'd
> create
> > 3 HTTP_LIVE delivery services? One with an origin in seattle. One with an
> > origin in denver. One with an origin in boston. (or would you create 6
> ds's
> > so you have 2 in each location?)
> >
> > Then you'd create a steering delivery service with 3 targets (the
> HTTP_LIVE
> > ds's you just created.) Each target would have type = GEO_ORDER and
> value =
> > ?
> >
> > I'm getting confused with that above scenario. How many targets would you
> > have? What would the type of each target be and what would the value of
> > each target be?
> >
> > Jeremy
> >
> >
> > On Fri, Feb 23, 2018 at 3:40 PM, Rawlin Peters <rawlin.pet...@gmail.com>
> > wrote:
> >
> >> Hey folks,
> >>
> >> At Comcast we have a need to augment CLIENT_STEERING (also regular
> >> STEERING while we're at it) to allow targets to be ordered/sorted
> >> based upon the client's proximity to the origin of the target delivery
> >> services. I'd like to get your feedback on my proposed design and
> >> address any of your concerns.
> >>
> >> For HTTP_LIVE targets for instance, we'd like edge caches to ideally
> >> retrieve/serve data from an Origin that is close to the client and
> >> fall back to Origins that are farther and farther away. This allows
> >> for increased redundancy while ordering optimal Origins (Delivery
> >> Services) for a client to choose from.
> >>
> >> For example, I have 3 Origins in different locations: Seattle, Denver,
> >> and Boston. I would create an HTTP_LIVE delivery service for each
> >> origin and a CLIENT_STEERING delivery service with those delivery
> >> services as targets. I would then like to have those targets ordered
> >> based upon proximity to the client. So a client in Seattle would get
> >> the list [Seattle, Denver, Boston], while a client in Boston would get
> >> the list [Boston, Denver, Seattle].
> >>
> >> To make things more complicated, I want to add a redundant origin in
> >> each location and split traffic between them (like STEERING_WEIGHT
> >> today) while taking into account the geo-ordering. I also want to be
> >> able to force an ordering (like STEERING_ORDER today) among co-located
> >> targets.
> >>
> >> In order to accomplish this I propose to:
> >> 1. add two new steering types: GEO_ORDER and GEO_WEIGHT (by adding a
> >> target of type GEO_*, a steering DS would then enable geo-ordering)
> >> 2. associate a Delivery Service Origin with a latitude/longitude,
> >> thereby associating a lat/long to a GEO_* target
> >>
> >> Item 1 is pretty straightforward and will also play nicely with the
> >> current steering types (STEERING_ORDER and STEERING_WEIGHT). I've
> >> completed a POC within Traffic Router that basically provides the
> >> following ordering when mixing all 4 types in a single steering
> >> delivery service:
> >> - Negative STEERING_ORDER targets
> >> - GEO_WEIGHT and GEO_ORDER targets, grouped by proximity to the
> >> client, ordered by geo-order and the consistent-hashing from the
> >> weightings
> >> - STEERING_WEIGHT targets (consistent-hashed)
> >> - Positive STEERING_ORDER targets
> >>
> >> Item 2 is not as straightforward because the simple thing would be to
> >> just add an Origin Lat/Long field to the Delivery Service and call it
> >> a day. However I don't think we should do that, and I'll dive more
> >> into that in a separate thread (coming soon).
> >>
> >> Does anyone have questions/concerns about adding these new GEO_ORDER
> >> and GEO_STEERING target types and geo-sorting them based upon
> >> proximity to the client? Are you okay with the proposed ordering when
> >> all the steering types are mixed together?
> >>
> >> - Rawlin
> >>
>

Re: Github PR/Issues Format Templates

[Jeremy Mitchell]

With an issue and/or pr template, we will have 6/6 items checked:

https://github.com/apache/incubator-trafficcontrol/community

I actually think PR templates would be quite helpful. As a
committer/merger, it would be nice to know what problem the PR is solving
and how to verify the functionality. A PR template could also help
contributors ensure that their PRs are complete. I.e. does this PR includes
tests, documentation, etc.

I'll take a stab at a couple of templates and run them by the group.

Jeremy

On Wed, Jan 31, 2018 at 1:10 PM, Chris Lemmons  wrote:

> I'm +1 on Issue Templates, for sure. I don't know that PR templates
> are quite as critical, but it might be nice to have a reminder in
> there about verifying that the changelog is updated if necessary and
> documentation for new features is present. If the PR Template
> overwrites the default comment that you get from the commit body, it
> might be more annoying than valuable, though.
>
> I'm also +1 on hiding these particular files in a .github directory.
> Unlike CONTRIBUTING and README, they don't provide all that much
> benefit for a new person looking for stuff to read.
>
> On Wed, Jan 31, 2018 at 12:17 PM, Durfey, Ryan 
> wrote:
> > Always +1 on standardization and consistency
> >
> > I still want to circle back and setup project/kanbans for organizing
> tickets in Github.
> >
> > Ryan DurfeyM | 303-524-5099
> > CDN Support (24x7): 866-405-2993 or cdn_supp...@comcast.com cdn_supp...@comcast.com>
> >
> > From: Dewayne Richardson 
> > Reply-To: "dev@trafficcontrol.incubator.apache.org" <
> dev@trafficcontrol.incubator.apache.org>
> > Date: Wednesday, January 31, 2018 at 11:15 AM
> > To: "dev@trafficcontrol.incubator.apache.org" <
> dev@trafficcontrol.incubator.apache.org>
> > Subject: Github PR/Issues Format Templates
> >
> > I was working through the go-swagger repo and found a bug.  I submitted a
> > new issue and found this interesting approach I think the TC github
> should
> > adopt, "Issue and PR Templates".  I think the main value here is
> > consistency in our PRs/Issues and user friendly prompts to say "these are
> > the data points we need to help you solve your issue".
> >
> > Working example:
> > https://github.com/go-swagger/go-swagger/issues/new
> >
> > Github Doc on how to implement templates:
> > https://github.com/blog/2111-issue-and-pull-request-templates
> >
> > If we think it's a good idea, then I'll respond with some examples for
> > Issues and PR's that we can discuss.
> >
> > -Dew
> >
>

Re: Steering Target Geo-Ordering

[Jeremy Mitchell]

So, back to the Seattle, Denver, Boston example...

To accomplish the geo ordering and redundant origin use case, you'd create
3 HTTP_LIVE delivery services? One with an origin in seattle. One with an
origin in denver. One with an origin in boston. (or would you create 6 ds's
so you have 2 in each location?)

Then you'd create a steering delivery service with 3 targets (the HTTP_LIVE
ds's you just created.) Each target would have type = GEO_ORDER and value =
?

I'm getting confused with that above scenario. How many targets would you
have? What would the type of each target be and what would the value of
each target be?

Jeremy


On Fri, Feb 23, 2018 at 3:40 PM, Rawlin Peters 
wrote:

> Hey folks,
>
> At Comcast we have a need to augment CLIENT_STEERING (also regular
> STEERING while we're at it) to allow targets to be ordered/sorted
> based upon the client's proximity to the origin of the target delivery
> services. I'd like to get your feedback on my proposed design and
> address any of your concerns.
>
> For HTTP_LIVE targets for instance, we'd like edge caches to ideally
> retrieve/serve data from an Origin that is close to the client and
> fall back to Origins that are farther and farther away. This allows
> for increased redundancy while ordering optimal Origins (Delivery
> Services) for a client to choose from.
>
> For example, I have 3 Origins in different locations: Seattle, Denver,
> and Boston. I would create an HTTP_LIVE delivery service for each
> origin and a CLIENT_STEERING delivery service with those delivery
> services as targets. I would then like to have those targets ordered
> based upon proximity to the client. So a client in Seattle would get
> the list [Seattle, Denver, Boston], while a client in Boston would get
> the list [Boston, Denver, Seattle].
>
> To make things more complicated, I want to add a redundant origin in
> each location and split traffic between them (like STEERING_WEIGHT
> today) while taking into account the geo-ordering. I also want to be
> able to force an ordering (like STEERING_ORDER today) among co-located
> targets.
>
> In order to accomplish this I propose to:
> 1. add two new steering types: GEO_ORDER and GEO_WEIGHT (by adding a
> target of type GEO_*, a steering DS would then enable geo-ordering)
> 2. associate a Delivery Service Origin with a latitude/longitude,
> thereby associating a lat/long to a GEO_* target
>
> Item 1 is pretty straightforward and will also play nicely with the
> current steering types (STEERING_ORDER and STEERING_WEIGHT). I've
> completed a POC within Traffic Router that basically provides the
> following ordering when mixing all 4 types in a single steering
> delivery service:
> - Negative STEERING_ORDER targets
> - GEO_WEIGHT and GEO_ORDER targets, grouped by proximity to the
> client, ordered by geo-order and the consistent-hashing from the
> weightings
> - STEERING_WEIGHT targets (consistent-hashed)
> - Positive STEERING_ORDER targets
>
> Item 2 is not as straightforward because the simple thing would be to
> just add an Origin Lat/Long field to the Delivery Service and call it
> a day. However I don't think we should do that, and I'll dive more
> into that in a separate thread (coming soon).
>
> Does anyone have questions/concerns about adding these new GEO_ORDER
> and GEO_STEERING target types and geo-sorting them based upon
> proximity to the client? Are you okay with the proposed ordering when
> all the steering types are mixed together?
>
> - Rawlin
>

Re: Traffic Ops API Swagger Doc

[Jeremy Mitchell]

Eric, are you saying you never want to write RST anymore? me neither. yuck!

On Tue, Feb 20, 2018 at 9:57 AM, Eric Friedrich (efriedri) <
efrie...@cisco.com> wrote:

> Is it possible to take the swagger generated documentation and have that
> automatically included in the read-the-docs site?
>
> Asked another way: Can swagger generate docs in ReStructed Text (.rst)
> format?
>
> —Eric
>
>
> > On Feb 20, 2018, at 11:38 AM, Dave Neuman <neu...@apache.org> wrote:
> >
> > Sounds good.  I look forward to seeing it merged into our repo.
> > I guess this means there will need to be a PR to remove our current API
> > docs as they get moved to swagger.
> >
> > On Tue, Feb 20, 2018 at 8:40 AM, Jeremy Mitchell <mitchell...@gmail.com>
> > wrote:
> >
> >> I think this all sounds very promising. Some advantages that I see are:
> >>
> >> - docs never drift from API implementation (currently our docs get out
> of
> >> sync real fast)
> >> - this provides yet another interface -
> >> https://app.swaggerhub.com/apis/dewrich/traffic-ops_api/1.3 -  (in
> >> addition
> >> to TP) to interact with the API
> >> - a great way to demonstrate / educate developers on the capabilities of
> >> the api
> >>
> >> plus, i heard some bonus features that could be really interesting:
> >>
> >> - auto generation of a TO golang client. can we deprecate the old TO
> client
> >> in favor of an auto-generated one? The current TO client never seems to
> >> stay in sync with the api
> >> - generating server stubs
> >>
> >> imo our api can never be fully auto-generated because of the business
> logic
> >> that needs to be accounted forbut it would be pretty neat if all we
> had
> >> to do was "define" the api in a yaml file and that yaml file would spit
> out
> >> documentation, tests, clients (be it golang or java or whatever), and
> >> server side code (stubs) and then all we could focus on writing code
> >> specific to business logic.
> >>
> >> my guess is to really do this right, we'd have to rev the api version to
> >> 2.0 to make the api more swagger friendly...tools (swagger) like
> >> consistency and our api is far from consistent...
> >>
> >> jeremy
> >>
> >> On Wed, Feb 14, 2018 at 10:08 AM, Durfey, Ryan <ryan_dur...@comcast.com
> >
> >> wrote:
> >>
> >>> I am +1 on the swagger concept.  This makes working with APIs much
> easier
> >>> for non-developer staff and makes it easier to educate customers as
> well.
> >>>
> >>> Ryan DurfeyM | 303-524-5099
> >>> CDN Support (24x7): 866-405-2993 or cdn_supp...@comcast.com >>> cdn_supp...@comcast.com>
> >>>
> >>> From: Dewayne Richardson <dewr...@gmail.com>
> >>> Reply-To: "dev@trafficcontrol.incubator.apache.org" <
> >>> dev@trafficcontrol.incubator.apache.org>
> >>> Date: Wednesday, February 14, 2018 at 9:34 AM
> >>> To: "dev@trafficcontrol.incubator.apache.org" <
> >>> dev@trafficcontrol.incubator.apache.org>
> >>> Subject: Traffic Ops API Swagger Doc
> >>>
> >>> We've been working diligently on the TO Golang Rewrite
> >>> <https://github.com/apache/incubator-trafficcontrol/projects/2>
> project
> >> to
> >>> obviously rewrite the Perl into Go, but also to improve our Testing and
> >>> Documentation efforts.  I presented the idea of using Swagger several
> >>> summits (years) ago about using Swagger to help drive our Traffic Ops
> API
> >>> documentation.  Since then Swagger has evolved and is becoming the de
> >> facto
> >>> standard for building (the potential for generating TO Golang Client
> and
> >>> Server stubs is now available) and documenting REST APIs.
> >>>
> >>> I would like to propose going forward that we use Swagger for future
> API
> >>> level documentation (because it can be generated out of our Golang
> >>> code/structs).  The below resources point out a TO API version 1.3 (the
> >>> version we decided to rev for the rewritten Golang endpoints).  The
> >> intent
> >>> behind 1.3 is obviously an improved version of the API (entirely
> backward
> >>> compatible to 1.2), but also to give us a starting point for building
> the
> >>> API doc in Swagger.
> &g

Re: Traffic Ops API Swagger Doc

[Jeremy Mitchell]

I think this all sounds very promising. Some advantages that I see are:

- docs never drift from API implementation (currently our docs get out of
sync real fast)
- this provides yet another interface -
https://app.swaggerhub.com/apis/dewrich/traffic-ops_api/1.3 -  (in addition
to TP) to interact with the API
- a great way to demonstrate / educate developers on the capabilities of
the api

plus, i heard some bonus features that could be really interesting:

- auto generation of a TO golang client. can we deprecate the old TO client
in favor of an auto-generated one? The current TO client never seems to
stay in sync with the api
- generating server stubs

imo our api can never be fully auto-generated because of the business logic
that needs to be accounted forbut it would be pretty neat if all we had
to do was "define" the api in a yaml file and that yaml file would spit out
documentation, tests, clients (be it golang or java or whatever), and
server side code (stubs) and then all we could focus on writing code
specific to business logic.

my guess is to really do this right, we'd have to rev the api version to
2.0 to make the api more swagger friendly...tools (swagger) like
consistency and our api is far from consistent...

jeremy

On Wed, Feb 14, 2018 at 10:08 AM, Durfey, Ryan 
wrote:

> I am +1 on the swagger concept.  This makes working with APIs much easier
> for non-developer staff and makes it easier to educate customers as well.
>
> Ryan DurfeyM | 303-524-5099
> CDN Support (24x7): 866-405-2993 or cdn_supp...@comcast.com cdn_supp...@comcast.com>
>
> From: Dewayne Richardson 
> Reply-To: "dev@trafficcontrol.incubator.apache.org" <
> dev@trafficcontrol.incubator.apache.org>
> Date: Wednesday, February 14, 2018 at 9:34 AM
> To: "dev@trafficcontrol.incubator.apache.org" <
> dev@trafficcontrol.incubator.apache.org>
> Subject: Traffic Ops API Swagger Doc
>
> We've been working diligently on the TO Golang Rewrite
>  project to
> obviously rewrite the Perl into Go, but also to improve our Testing and
> Documentation efforts.  I presented the idea of using Swagger several
> summits (years) ago about using Swagger to help drive our Traffic Ops API
> documentation.  Since then Swagger has evolved and is becoming the de facto
> standard for building (the potential for generating TO Golang Client and
> Server stubs is now available) and documenting REST APIs.
>
> I would like to propose going forward that we use Swagger for future API
> level documentation (because it can be generated out of our Golang
> code/structs).  The below resources point out a TO API version 1.3 (the
> version we decided to rev for the rewritten Golang endpoints).  The intent
> behind 1.3 is obviously an improved version of the API (entirely backward
> compatible to 1.2), but also to give us a starting point for building the
> API doc in Swagger.
>
> The following resources are my examples:
>
> Swagger has several implementations and I chose go-swagger
>  because it has more Golang
> features.
>
> *Sample TO API doc *
>
> https://app.swaggerhub.com/apis/dewrich/traffic-ops_api/1.3
>
>
> *Sample TO Golang code with embedded doc*
>
> Generated from the combination of these Golang documentation "hooks"
> (there's current a go-swagger bug that prevents the doc from being tied
> directly into the handlers)
> https://github.com/dewrich/incubator-trafficcontrol/tree/
> swagger-demo/traffic_ops/traffic_ops_golang/docs
>
> And the *asns.go*, *cdns.go*, *divisions.go*, *regions.go* and
> *statuses.go*
> structs in my branch here:
> https://github.com/dewrich/incubator-trafficcontrol/tree/
> swagger-demo/lib/go-tc
>
>
> *TO Client Generated from Swagger*
>
> This new Golang package is only a sample of a TO client generated (based
> upon the the code generated swagger.json
>  trafficcontrol/blob/swagger-demo/traffic_ops/traffic_ops_
> golang/docs/swagger.json dewrich/incubator-trafficcontrol/blob/swagger-
> demo/traffic_ops/traffic_ops_golang/docs/swagger.json>>
> )
>
> https://github.com/dewrich/incubator-trafficcontrol/tree/
> swagger-demo/traffic_ops/traffic_ops_golang/toclient
> https://github.com/dewrich/incubator-trafficcontrol/tree/
> swagger-demo/traffic_ops/traffic_ops_golang/toclient/main.go
>
> The hope with tying the documentation closer to the code will help with
> keeping the API docs up-to-date, as well as providing more documentation
> for developers.
>
> Please give your thoughts on this idea as well as a vote by Feb 21 (a week
> from today) so that we can move forward with building our TO API doc.
>
> -Dew
>
>

Re: New Self-Service Initiative - Delivery Service Request System

[Jeremy Mitchell]

If you haven't had a chance to read https://cwiki.apache.org/
confluence/display/TC/Delivery+Service+Requests, I would recommend it.

Self-service is quite a large undertaking so to make it more digestible,
we've created the notion of "delivery service requests". Think of it like
PRs (or pull requests) for delivery services. When enabled, all delivery
service changes submitted in the Traffic Portal (create, update and delete)
are captured as requests that need to be fulfilled by an ops/admin user.
When disabled, all delivery service changes follow the normal flow and
currently require the ops/admin role.

There are a few advantages to this approach:

1. Because they are only "requests", we can allow users with non-elevated
permissions (ops/admin) to create these requests.
2. We can provide pseudo self-service in the short term while end-to-end,
automated self-service is designed and built.
3. Like a PR, there are 2 participants (in this case requester and
fulfiller). More participants == less errors.
4. There is a history of changes for a delivery service, who requested them
and who fulfilled them.

Here's an example of the workflow:

1. User submits a request to create a new delivery service
2. Ops/admin user reviews and fulfills the request (which creates the
delivery service and marks the request as "pending")
3. Ops/admin user does additional things like

   - assign caches to the ds
   - queue updates
   - snapshot config

4. Ops/admin user manually marks request as 'complete'

The DS has now been created and the user can attach SSL keys, manage
steering targets or just use the DS.

There are a few manual steps here as you can see and we need to remove
those (i.e. queue updates and snapshot). That will be the bigger challenge.

There will be some debate I'm sure whether these "delivery service
requests" will be part of the longer term solution but in my opinion, I
don't see why not.

Longer term example workflow:

1. User submits a request to create a new delivery service
2. Something validates the ds config (maybe a CI process?)
3. If validation was successful, changes are persisted to the database
4. Request is marked as 'complete'

No more human intervention.

Here are 2 PR's that have been submitted for this initiative. Comments,
concerns welcome.

API (traffic ops) -
https://github.com/apache/incubator-trafficcontrol/pull/1888
UI (traffic portal) -
https://github.com/apache/incubator-trafficcontrol/pull/1818

Jeremy







On Thu, Dec 14, 2017 at 4:00 PM, Durfey, Ryan 
wrote:

> We are starting on building a “Delivery Service Request” system into the
> Traffic Portal UI.  This will start as a manual system to streamline new
> service builds using user inputs and manual operations review and system
> updates.  This system will evolve over time into full self-service with
> automated validation and deployment.
>
> Initial Notes: https://cwiki.apache.org/confluence/display/TC/
> Delivery+Service+Requests
>
> Once we get a bit more organized and documented we will circulate concepts
> for comment.
>
> Ryan Durfey
> Sr. Product Manager - CDN | Comcast Technology Solutions
> 1899 Wynkoop Ste. 550 | Denver, CO 80202
> M | 303-524-5099
> ryan_dur...@comcast.com
> CDN Support (24x7): 866-405-2993 or cdn_supp...@comcast.com cdn_supp...@comcast.com>
>
>

Re: Immutable DS CDN - resolving Riak/Postgres data coherency

[Jeremy Mitchell]

First, I'm +100 on doing a riak clean-up on DS delete but not a partial
cleanup (just latest) but a full cleanup (ALL SSL keys (if applicable), ALL
url sig keys (if applicable), ALL URI signing keys (if applicable)).
Otherwise, when a DS gets created down the road with that same xml-id,
problems may arise.

As far as making the CDN immutable...there has been some discussion around
mapping a tenant to one or more CDNs. This is because of self-service. When
a SS user is creating a DS, is it fair to ask them to choose a CDN? What if
you have 10 CDNs defined? They will have no idea what CDN to choose.
Therefore, it makes more sense to ask them to choose a tenant for their DS.
Tenants in most cases will have a one-to-one mapping to CDN so the CDN will
be selected for them. So, if we make CDN immutable, that means tenant is
now immutable...

Plus, let's face it, mistakes happen. It would suck to create a DS and get
it all setup only to discover you selected the wrong CDN and have to start
over. I think we should explore what it would take to make the system
handle CDN changes more gracefully.

Jeremy



On Wed, Feb 14, 2018 at 8:22 AM, Nir Sopher  wrote:

> See WIP PR:
> https://github.com/apache/incubator-trafficcontrol/pull/1868/files
> Deleting only the latest
>
> On Wed, Feb 14, 2018 at 4:56 PM, Steve Malenfant 
> wrote:
>
> > Would deleting the certificate only remove the "latest" copy/alias? The
> > certificate and keys should still be retrievable manually.  Yes/No?
> >
> > On Tue, Feb 13, 2018 at 5:40 PM, Dave Neuman  wrote:
> >
> > > I think I can get on board with not allowing a user to change the CDN.
> > If
> > > you want to change the CDN you need to delete your DS and re-create it
> or
> > > create a new DS with a different XML_ID and a regex that matches the
> > first
> > > DS.
> > >
> > > We have gone back and forth several times on deleting the keys from
> riak
> > > when you delete a DS.  Each time we decide not to make the change for
> one
> > > reason or another.  The worry is that if you delete a DS and then
> decide
> > > that it was a mistake you now have to generate a whole new certificate
> > > which could cost real money.  I am not sure that use-case is common
> > enough
> > > to warrant us not deleting the certificates for a DS.  For now I am +1
> on
> > > deleting the certificates when a DS is deleted.
> > >
> > > Thanks,
> > > Dave
> > >
> > > On Tue, Feb 13, 2018 at 12:14 PM, Nir Sopher  wrote:
> > >
> > > >  Hi,
> > > >
> > > > I created a delivery service and later on realized it is in the wrong
> > > CDN.
> > > > I then changed the CDN.
> > > > The ssl-keys record in the riak kept referring to the old CDN, even
> if
> > I
> > > > generated new certificates. Traffic router was therefore unable to
> pull
> > > the
> > > > certificate.
> > > >
> > > > See issue 1847
> > > > 
> > > >
> > > > A fix to this issue can be done by changing the code so the record in
> > the
> > > > Riak is updated along with the DS update.
> > > > However, this does not really make sense - if the CDN has changed,
> the
> > > > domain usually changes as well and the certificate is no longer
> valid.
> > > >
> > > > Therefore, I suggest to entirely block DS CDN change [see
> > > > https://github.com/apache/incubator-trafficcontrol/pull/1872]
> > > > .
> > > > Additionally, I added a PR for ssl-keys deletion up-on DS deletion,
> so
> > > > deleting the DS and recreating it would not cause similar issues.
> > > >
> > > > Would appreciate community input for other alternatives.
> > > >
> > > > Thanks,
> > > > Nir
> > > >
> > >
> >
>

Re: Delivery Service Self-Service

[Jeremy Mitchell]

Thanks for all the input. Here's what I heard. For a refresher, these are
the 7 things that a "non-ops" user should be able to perform to achieve
"delivery service self-service":

1. CRUD delivery services - ✓

- non-ops users will be able to create "delivery service requests". ops
users will be able to fulfill those requests. so indirectly, non-ops users
can CRUD ds's. I will be creating documentation around this shortly.

2. Manage DS regexes - X

- this action should continue to be limited to ops users...however, maybe
we allow non-ops users to create cnames and cnames only that don't contain
regex values but conform to hostname format only. more research needed here.

3. Manage SSL keys - ✓

- non-ops users should be able to manage (add, generate) ssl keys for their
delivery services.

4. Manage URL sig keys - ✓

- non-ops users can already do this for their delivery services

5. Manage URI signing keys - X

- this action should continue to be limited to ops users as it could be
dangerous at the moment (i.e. bad neighbor behavior). need more research
here.

6. Manage steering targets - ✓

- Tenancy dictates which delivery services can be defined as a target
therefore no risk to allowing non-ops users to manage steering targets.
(they can only point a steering ds to their own ds's)

7. Invalidate DS content - ✓

- non-ops users can already do this for their delivery services

Not bad. 5 out of 7.

I will follow up later regarding "Manage DS regexes" and "Manage URI
signing keys" so we can figure out a way to provide that functionality to
non-ops users...

Thanks again,

Jeremy



On Mon, Feb 5, 2018 at 12:26 PM, Nir Sopher <n...@qwilt.com> wrote:

> Hi Jeremy and all,
>
> Jeremy, thanks for putting it all together!
>
> I'll be short, as I mostly agree with most of point the Jeremy's pointed
> out.
>
> Regarding the "DS regex", like Ryan IIUC, I believe, the operator needs to
> be able to configure it and control it.
> First of all as it defines a resource in the operator's domain.
> Second, the definition of the regex requires some clear methodology to
> avoid collisions, or reuse/abuse.
> Following Rawlin remark, some reasonable default should be provided, but we
> must have the ability to change it.
> Note that path regexs are important as DS identifiers, supporting the SVA
> open-caching scheme.
>
> For the last point, the DS/Server assignment, I believe it should be in the
> hands of the operator.
> In the future it can be delegated the the user, subject to capacity
> management. The user should not be familiar with the actual caches, but can
> use some filters/tagging for defining the caches to be used.
>
> Nir
>
> On Mon, Feb 5, 2018 at 8:23 PM, Rawlin Peters <rawlin.pet...@gmail.com>
> wrote:
>
> > Replies inline
> >
> > On Fri, Feb 2, 2018 at 1:43 PM, Jeremy Mitchell <mitchell...@apache.org>
> > wrote:
> > > 2. Manage DS regexes
> > >
> > > Here's an explanation of this:
> > > http://traffic-control-cdn.readthedocs.io/en/latest/
> > admin/traffic_ops/using.html#delivery-service-regexp
> > >
> > > Currently, this requires the Operations role and for good reason. The
> > > danger here involves the risk of a normal user entering a bad regex.
> For
> > > example, it is my understanding that the regex in position zero needs
> to
> > > always follow this format: .*\.foo\..*.
> > >
> > > Maybe with some better API validation we could let normal users manage
> DS
> > > regexesor maybe these end up going away in favor of something
> > > better/easier...not sure yet...
> >
> > I think the approach that the Traffic Portal takes today is good. Just
> > giving a DS a default HOST regex with order = 0 using the xml_id will
> > probably cover most use cases for the DS. Then for the cases where
> > someone is CNAMEing to the DS FQDN, the DS owner should be able to add
> > a max number of HOST regexes with order > 0 matching the CNAME fqdn.
> > We should probably just call those "CNAME aliases" or something and
> > just expose them as a simple hostname list in the UI rather than as
> > HOST regexes with a specific ordering. For a list of aliases, I don't
> > think the order really matters at that point as long as they're
> > greater than 0 and sequential. That operation could be safe for a
> > regular DS owner assuming we validate that the alias is a valid
> > hostname (not a regex), unique, and not in use anywhere else in that
> > CDN.
> >
> > We might want to prohibit creating multiple HOST regexes with wildcard
> > characters (i.e. non-CNAME-alias)...I'm not even sure there's a v

Delivery Service Self-Service

[Jeremy Mitchell]

As we move in the direction of self-service, there are a few obstacles that
need to be overcome and I'd like to discuss them a bit so grab a cup of
coffee...

When I say self-service, what I really mean is "delivery service
self-service" or the ability to manage your own delivery services (as
dictated by tenancy) and everything related to those delivery services.
"Everything" includes the following (afaik):

1. Create/Read/Update/Delete delivery services
2. Manage DS regexes
3. Manage DS SSL keys (if applicable)
4. Manage DS URL sig keys (if applicable)
5. Manage DS URI signing keys (if applicable)
6. Manage DS targets (steering* only)
7. Creating DS invalidate content jobs
8. Manage DS / cache assignments

If you can't do 1-7 yourself, it's not really self-service is it? #8 is
debatable.

I'll discuss each one:

1. Create/Read/Update/Delete delivery services

Ideally, you could CRUD your own delivery services but our system has some
limitations.

A) Our CDN is not properly insulated from bad DS configurations. If a user
enters a bad value, bad things could potentially happen to a cache or worse
the whole CDN.
B) Certain DS configuration changes requires queue updates and/or snapshot
for the change to take effect. We're not ready (nor will we ever be
probably) to let normal users queue updates and/or snapshot.

So in the interim, we're working on the ability to allow normal users to
create "delivery service requests" to facilitate creating/updating/deleting
a delivery service. These "requests" will have to be reviewed/fulfilled by
a higher level user (Ops or Admin) who can then queue/snapshot if needed.

2. Manage DS regexes

Here's an explanation of this:
http://traffic-control-cdn.readthedocs.io/en/latest/admin/traffic_ops/using.html#delivery-service-regexp

Currently, this requires the Operations role and for good reason. The
danger here involves the risk of a normal user entering a bad regex. For
example, it is my understanding that the regex in position zero needs to
always follow this format: .*\.foo\..*.

Maybe with some better API validation we could let normal users manage DS
regexesor maybe these end up going away in favor of something
better/easier...not sure yet...

3. Manage DS SSL keys

SSL keys are only applicable where protocol > 0 (HTTPS, HTTP AND HTTPS, or
HTTP TO HTTPS) and currently, to manage them requires the Admin role. Why?
I'm not sure. Is their harm in letting normal users manage their own SSL
keys?

4. Manage DS URL sig keys

URL sig keys (
http://traffic-control-cdn.readthedocs.io/en/latest/admin/traffic_ops/using.html#generate-url-sig-keys)
are only applicable where signingAlgorithm = 'url_sig' and currently, to
manage them requires NO role apparently (only a tenancy check is
performed). This is the 1st one on the list that a "normal" user can do
currently.

5. Manage DS URI signing keys

URI signing keys are only applicable where signingAlgorithm = 'uri_signing'
and currently, to manage DS URI signing keys requires the Admin role. Is it
necessary to restrict this functionality to Admins only or can we allow
normal users to mange URI signing keys for their DS's?

6. Manage DS targets (steering* only)

Here's an explanation of this:
http://traffic-control-cdn.readthedocs.io/en/latest/admin/quick_howto/steering.html?highlight=steering

Currently, to manage DS targets requires the Admin or Steering role. Is
there any harm in allowing a normal user to "steer" their delivery service
to another delivery service as long as the target delivery service falls in
their tenancy?

7. Creating DS invalidate content jobs

http://traffic-control-cdn.readthedocs.io/en/latest/admin/traffic_ops/using.html?highlight=invalidate#invalidate-content

You can currently do this for your own DS's. This is the 2nd one on the
list that a "normal" user can do currently.

8. Manage DS / cache assignments

This is the debatable one. To provide true delivery service self-service
should a user have the ability to determine which caches are assigned to
their delivery service. I'm thinking NO. Currently, this action requires
the Ops role and I'm in favor of leaving it that way...

In summary, to provide true delivery service self-service I think we need
to do a few things:

1. Introduce DS requests until the day in which DS configurations can be
guaranteed and queue updates/snapshot becomes a thing of the past. (this is
in progress)
2. Revisit DS regexes or make their management more fool proof.
3. Tweak the roles of these actions. Currently, a lot of these things are
reserved for Ops/Admin. We have to change that or full DS self-service if
not possible. I'd like to make each of these things accessible to users
with the "Portal" role with the exception of #8.

Thoughts? Concerns? Funny jokes?

Jeremy

Re: [VOTE] CHANGELOG.md file (second try)

[Jeremy Mitchell]

ed then it is to try to take a list of PRs
> >> and
> >>>> turn it into a high level list of what's important.
> >>>>
> >>>> We need a solution that gives someone what they need at a glance.
> >>> Something
> >>>> that can be copy/pasted out or easily linked to.  IMO not all of our
> >>> users
> >>>> are super technical or involved in the day to day so we shouldn't just
> >>>> point them at a list of PRs and say "figure it out"; we should make it
> >>> easy
> >>>> for them to figure out.
> >>>>
> >>>> I have seen lots of alternatives suggested to what Steve has proposed,
> >>> but
> >>>> I haven't seen anyone state why they don't like what Steve has
> >>> proposed?  I
> >>>> think what he is proposing is pretty standard across major Github
> >>> projects
> >>>> that I have seen.  I understand that we can introduce some additional
> >>>> inconvenience with having to manually write what your feature or bug
> >> fix
> >>>> does, and there could be some conflicts, but isn't it important to
> >>> clearly
> >>>> portray what has changed?  I don't think we need a changelog.md entry
> >> to
> >>>> every PR, in fact I hope we don't...we need a changelog.md entry any
> >>> time
> >>>> we add a new feature, any time we have some operational considerations
> >>> that
> >>>> need to be communicated, or any time that we fix a bug from a previous
> >>>> release.
> >>>>
> >>>>
> >>>> On Thu, Dec 14, 2017 at 2:02 PM, Jeremy Mitchell <
> >> mitchell...@gmail.com>
> >>>> wrote:
> >>>>
> >>>>> This label idea would require everyone to go back and find their PRs
> >>> that
> >>>>> were closed after Aug 4th (the date 2.1 branch was created) and
> >> attach
> >>>> the
> >>>>> 'change log' label and the 2.2 milestone to the appropriate ones,
> >>> right?
> >>>>> And then that link dew provide would be an accurate picture of what
> >> has
> >>>>> changed between 21. and 2.2...
> >>>>>
> >>>>> of course, ignore PRs that don't affect the "interface" like "license
> >>>>> changes", right?
> >>>>>
> >>>>> i like the idea...
> >>>>>
> >>>>> On Thu, Dec 14, 2017 at 1:59 PM, Dewayne Richardson <
> >> dewr...@gmail.com
> >>>>
> >>>>> wrote:
> >>>>>
> >>>>>> As a POC for the Change Log label follow this link:
> >>>>>>
> >>>>>> https://github.com/apache/incubator-trafficcontrol/
> >>>>>> pulls?q=is%3Apr+is%3Aclosed+label%3A%22change+log%22+
> >>> milestone%3A2.2.0
> >>>>>>
> >>>>>> -Dew
> >>>>>>
> >>>>>> On Thu, Dec 14, 2017 at 10:48 AM, Gelinas, Derek <
> >>>>>> derek_geli...@comcast.com>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> I'm +1 for the label as well.
> >>>>>>>
> >>>>>>>> On Dec 14, 2017, at 12:29 PM, Robert Butts <
> >>>> robert.o.bu...@gmail.com
> >>>>>>
> >>>>>>> wrote:
> >>>>>>>>
> >>>>>>>> +1 on a "changelog" label. Seems like that would make it a lot
> >>>> easier
> >>>>>> for
> >>>>>>>> the person writing it up. Easier to skip things like code
> >>>> maintenance
> >>>>>>> that
> >>>>>>>> have no interface effect.
> >>>>>>>>
> >>>>>>>> On Thu, Dec 14, 2017 at 10:14 AM, Dewayne Richardson <
> >>>>>> dewr...@gmail.com>
> >>>>>>>> wrote:
> >>>>>>>>
> >>>>>>>>> Another idea would be to include a new CHANGELOG label that
> >> you
> >>>>> could
> >>>>>>> tack
> >>>>>>>>> onto specific PR's that you wanted to be included (as well as
> >>>>> grouped
>

Re: [VOTE] Release Apache Traffic Control (incubating) 2.1.0-RC3

[Jeremy Mitchell]

+1

On Wed, Dec 20, 2017 at 5:34 PM, Steve Malenfant 
wrote:

> +1
>
> On Wed, Dec 20, 2017 at 5:14 PM, Dave Neuman  wrote:
>
> > +1
> >
> > On Wed, Dec 20, 2017 at 8:33 AM, Hank Beatty  wrote:
> >
> > > Hello All,
> > >
> > > I've prepared a release for v2.1.0-RC3
> > >
> > > The vote is open for at least 72 hours and passes if a majority of at
> > > least 3 +1 PPMC votes are cast.
> > >
> > > [ ] +1 Approve the release
> > >
> > > [ ] -1 Do not release this package because ...
> > >
> > > Changes since 2.0.0:
> > > https://github.com/apache/incubator-trafficcontrol/compare/
> > > 2.0.x...RELEASE-2.1.0-RC3
> > >
> > > This corresponds to git:
> > >   Hash: 1dcd512f7e2b4898b090837cd3f260e453896e32
> > >   Tag: RELEASE-2.1.0-RC3
> > >
> > > Which can be verified with the following: git tag -v RELEASE-2.1.0-RC3
> > >
> > > My code signing key is available here:
> > > https://pgp.mit.edu/pks/lookup?op=get=0x920152B94E0CC77C
> > >
> > > The source .tar.gz file, pgp signature (.asc signed with my key from
> > > above), md5 and sha512 checksums are provided here:
> > >
> > > https://dist.apache.org/repos/dist/dev/incubator/
> > trafficcontrol/2.1.0/RC3
> > >
> > > The new proposed download page can be found here:
> > >
> > > https://trafficcontrol.incubator.apache.org/downloads/index-new.html
> > >
> > > Thanks!
> > >
> >
>

Re: [VOTE] CHANGELOG.md file (second try)

[Jeremy Mitchell]

This label idea would require everyone to go back and find their PRs that
were closed after Aug 4th (the date 2.1 branch was created) and attach the
'change log' label and the 2.2 milestone to the appropriate ones, right?
And then that link dew provide would be an accurate picture of what has
changed between 21. and 2.2...

of course, ignore PRs that don't affect the "interface" like "license
changes", right?

i like the idea...

On Thu, Dec 14, 2017 at 1:59 PM, Dewayne Richardson <dewr...@gmail.com>
wrote:

> As a POC for the Change Log label follow this link:
>
> https://github.com/apache/incubator-trafficcontrol/
> pulls?q=is%3Apr+is%3Aclosed+label%3A%22change+log%22+milestone%3A2.2.0
>
> -Dew
>
> On Thu, Dec 14, 2017 at 10:48 AM, Gelinas, Derek <
> derek_geli...@comcast.com>
> wrote:
>
> > I'm +1 for the label as well.
> >
> > > On Dec 14, 2017, at 12:29 PM, Robert Butts <robert.o.bu...@gmail.com>
> > wrote:
> > >
> > > +1 on a "changelog" label. Seems like that would make it a lot easier
> for
> > > the person writing it up. Easier to skip things like code maintenance
> > that
> > > have no interface effect.
> > >
> > > On Thu, Dec 14, 2017 at 10:14 AM, Dewayne Richardson <
> dewr...@gmail.com>
> > > wrote:
> > >
> > >> Another idea would be to include a new CHANGELOG label that you could
> > tack
> > >> onto specific PR's that you wanted to be included (as well as grouped
> > >> within Milestones) as the high level features that went into the
> > release.
> > >> As for the gotchas, I think those should be Github issues because to
> me
> > >> those were bugs.
> > >>
> > >> -Dew
> > >>
> > >> On Thu, Dec 14, 2017 at 10:01 AM, Jeremy Mitchell <
> > mitchell...@gmail.com>
> > >> wrote:
> > >>
> > >>> I agree. Very readable. I also like the idea of a either creating a
> > >>> CHANGELOG.md for each component...or one CHANGELOG.md with sections
> for
> > >>> each component.
> > >>>
> > >>> I still do like the idea of creating issues with milestones but I'll
> > let
> > >>> that go. That seems to be a personal preference of mine.
> > >>>
> > >>> Jeremy
> > >>>
> > >>>> On Thu, Dec 14, 2017 at 9:45 AM, Dave Neuman <neu...@apache.org>
> > wrote:
> > >>>>
> > >>>> Have you taken a look at some Changelogs of other github projects?
> > >> Here
> > >>>> are a few from other projects we use in Traffic Control:
> > >>>> - Docker Compose: https://github.com/docker/
> > >>> compose/blob/master/CHANGELOG.
> > >>>> md
> > >>>> - InfluxDB: https://github.com/influxdata/influxdb/blob/master/
> > >>>> CHANGELOG.md
> > >>>> - Grafana: https://github.com/grafana/grafana/blob/master/CHANGELOG
> .
> > md
> > >>>> - Ansible: https://github.com/ansible/ansible/blob/devel/CHANGELOG.
> md
> > >>>>
> > >>>> See how easy to read those are and how they provide a lot of
> > >> information
> > >>>> without having to wade through hundreds of issues and pull requests?
> > >>> Some
> > >>>> of them also have links to issues for new features, as well as bug
> > >> fixes
> > >>>> that are in the current release.  I think all of them are easy to
> read
> > >>> and
> > >>>> can give a user a quick overview of what is in the new release. I
> > think
> > >>>> it's fine to add a link to all the issues if we want, but I think
> > >>>> ultimately we want to create something like what I have linked to
> > >> above.
> > >>>> We might also want to break out our CHANGELOG.md by component to
> > >> provide
> > >>>> even more readability.
> > >>>>
> > >>>> I know our first inclination is to try to automate everything, but
> > >>>> sometimes it makes sense to do things manually so that you can
> create
> > a
> > >>>> better output.
> > >>>>
> > >>>>
> > >>>>
> > >>>> On Thu, Dec 14, 2017 at 8:55 AM, Jeremy Mitchell <
> > >> mitchell...@gmail.com>
> > >>>> wrote:
> > >>>>

Re: [VOTE] CHANGELOG.md file (second try)

[Jeremy Mitchell]

What if CHANGLOG.md includes 2 things:

1. a link to 2.2 issues (i.e.
https://github.com/apache/incubator-trafficcontrol/issues?q=is%3Aopen+is%3Aissue+milestone%3A2.2.0
2. a bulleted list of 2.2 gotchas (i.e. Traffic Ops Golang doesn't connect
to a Riak with self-signed certificates; Riak security grant needs updated;
upgrade param needed for ds routing names, etc, etc...)

But again this requires everyone to create issues (with a milestone) in
which one or more PR's can be attached to.

Dave, you said "If you are developing a new feature you could easily end up
with 5 or more PRs"

So in this case, I'd expect 1 issue and 5 PR's linked to that 1 issue...

Jeremy

On Thu, Dec 14, 2017 at 8:40 AM, Dave Neuman <neu...@apache.org> wrote:

> I think it's fine to include all of our PRs and issues in a github
> milestone, and we should do that, but I don't think that we want to include
> every single PR in our changelog.  When we have tried that in the past we
> have realized that it gets to be so much information that it's not useful.
> A good example of why is a new feature. If you are developing a new feature
> you could easily end up with 5 or more PRs: one to introduce the feature,
> one to add some more functionality, several to fix bugs with it, etc.
> Instead of having a line in the changelog for each one of those PRs, we
> should just have one line that says "introduced feature X" with maybe a
> blurb about what it is and any release notes that an operator would need to
> know.  This way the file in concise and easy to understand by anyone
> wanting to use a new version of our product.  I think it's also important
> to include what bug fixes (since the previous release) that we have fixed.
> Those are the reasons why I tend to lean towards a manual changelog.  It
> gives us the ability to control how much information goes into the
> changelog and the flexibility to make sure it is useful.
>
> On Thu, Dec 14, 2017 at 8:13 AM, Jeremy Mitchell <mitchell...@gmail.com>
> wrote:
>
> > Why not leverage Github issues/milestones to determine the scope of each
> > release? Per our CONTRIBUTING.md
> > <https://github.com/apache/incubator-trafficcontrol/blob/
> > master/CONTRIBUTING.md>
> > :
> >
> > "If you have improvements (enhancements or bug fixes) for Traffic
> Control,
> > start by creating an issue
> > <https://github.com/apache/incubator-trafficcontrol/issues>..."
> >
> > That implies that all PR's should be mapped to an issue although we do
> not
> > enforce this but if we did it would be easy to put each issue into a
> > milestone (2.2 for example) and then pull a github report at any time.
> >
> > Orrather than creating an issue, put a good title/description on your
> > PR and then the PR can be assigned to the milestone instead.
> >
> > It just seems like that's what milestones are for so why not use them?
> >
> > Jeremy
> >
> >
> >
> > On Wed, Dec 13, 2017 at 3:28 PM, Dave Neuman <neu...@apache.org> wrote:
> >
> > > I am +1 on this approach, I know it's manual and there could be
> conflicts
> > > and it's a bit of a PITA, but I think it actually has some benefits in
> > that
> > > a human can actually enter in important release notes that are readable
> > and
> > > make sense.
> > > That being said if someone is willing to make an automated approach
> work,
> > > that allows us to keep track of changes at a reasonable level (not per
> > > commit, not necessarily even per PR), then I could be +1 on that as
> well.
> > > I would need to someone volunteer to make it work before I give my +1
> > > though.
> > > Either way, we REALLY need a changelog that is updated regularly with
> > > important information.
> > >
> > > --Dave
> > >
> > > On Wed, Dec 13, 2017 at 3:00 PM, Steve Malenfant <smalenf...@gmail.com
> >
> > > wrote:
> > >
> > > > Hey All,
> > > >
> > > > There has been a vote on not maintaining a CHANGELOG file in the past
> > and
> > > > seems like we leaned toward an automated process. I believe none of
> > them
> > > > had happened (please correct me if not).
> > > >
> > > > I have been upgrading Traffic Control from 2.1 to 2.2 this week and
> > found
> > > > numerous gotchas.
> > > >
> > > > Some examples of things I ran into and luckily I was able to get good
> > > > support from the Slack channels. Here is a few example of possible
> > > breaking
> > > > changes :
>

Re: [VOTE] CHANGELOG.md file (second try)

[Jeremy Mitchell]

Why not leverage Github issues/milestones to determine the scope of each
release? Per our CONTRIBUTING.md

:

"If you have improvements (enhancements or bug fixes) for Traffic Control,
start by creating an issue
..."

That implies that all PR's should be mapped to an issue although we do not
enforce this but if we did it would be easy to put each issue into a
milestone (2.2 for example) and then pull a github report at any time.

Orrather than creating an issue, put a good title/description on your
PR and then the PR can be assigned to the milestone instead.

It just seems like that's what milestones are for so why not use them?

Jeremy



On Wed, Dec 13, 2017 at 3:28 PM, Dave Neuman  wrote:

> I am +1 on this approach, I know it's manual and there could be conflicts
> and it's a bit of a PITA, but I think it actually has some benefits in that
> a human can actually enter in important release notes that are readable and
> make sense.
> That being said if someone is willing to make an automated approach work,
> that allows us to keep track of changes at a reasonable level (not per
> commit, not necessarily even per PR), then I could be +1 on that as well.
> I would need to someone volunteer to make it work before I give my +1
> though.
> Either way, we REALLY need a changelog that is updated regularly with
> important information.
>
> --Dave
>
> On Wed, Dec 13, 2017 at 3:00 PM, Steve Malenfant 
> wrote:
>
> > Hey All,
> >
> > There has been a vote on not maintaining a CHANGELOG file in the past and
> > seems like we leaned toward an automated process. I believe none of them
> > had happened (please correct me if not).
> >
> > I have been upgrading Traffic Control from 2.1 to 2.2 this week and found
> > numerous gotchas.
> >
> > Some examples of things I ran into and luckily I was able to get good
> > support from the Slack channels. Here is a few example of possible
> breaking
> > changes :
> >
> > - Delivery Service prefixes disappeared after upgrade, was not handled in
> > postinstall (requires special attention, this was on this forum and well
> > documented on the mailing list)
> > - Traffic Ops Golang doesn't connect to a Riak with self-signed
> > certificates
> > - Riak security grant needs updated
> > - cdn.conf configuration change
> > - Traffic Portal required for new features (URI Signing)
> > - cachekey plugin instead of cacheurl
> >
> > There was great enhancements made in 2.2 needs to be noticed by current
> and
> > new users.  If we are looking to get more engagement, that's probably the
> > #1 thing to do. I usually go and read about all the other components
> which
> > we use around the Traffic Control CDN (Influx, Elastic, Grafana, etc...)
> >
> > So let me re-quote what Dave has sent and ask the same question again.
> >
> > ==
> > Hey All,
> > One thing we discussed at the meetup was the addition of a CHANGELOG.md
> > file to the project.   This file will contain changes that are made to
> the
> > project including bug fixes and new features. (e.g.
> > https://github.com/influxdata/influxdb/blob/master/CHANGELOG.md).
> Adding
> > this file means that we will now require each PR to contain an update to
> > the CHANGELOG.md file, and our documentation will need to be updated
> > accordingly.
> > I thought it would be good to open a vote for adding this file, and if it
> > passes, I will update the documentation and add a CHANGELOG.md file.
> >
> > Thanks,
> > Dave
> > ==
> >
> > I'm a +1 on CHANGELOG, but I'm not heavy creating PRs which kind
> influence
> > my vote.
> >
> > Steve
> >
>

VISION.md

[Jeremy Mitchell]

At Comcast, we've discussed the need for a document that outlines the
short/long-term goals of TC. Rather than calling this a roadmap which
implies time/deadlines, we thought VISION.md might be appropriate.

I imagine VISION.md will contain (for now) a simple, bulleted list of goals
such as:

- Traffic Portal feature parity with TO UI
- Deprecation of the TO UI (and the associated UI namespace/routes)
- Enhanced TO Roles/Capabilities
- TO/TP Self-Service
- TO API Golang Rewrite
- Cache Agnostification
- etc

Maybe we just start with a bulleted list and later a short description can
be added to each item.

What else can I add to this list? Also, I envision this is a living
document that everyone can contribute to via PRs (as long as the larger
group agrees that the addition is inline with the overall vision of TC).

Thoughts?

Jeremy

Scoping a release

[Jeremy Mitchell]

At the Traffic Control summit, we discussed the need to do a better job
scoping releases. What changes can I expect in an upcoming release? What
changes went in to a completed release?

The only way I can think to do this (without having a release manager that
actively manages the scope of a release) is to leverage Github labels and
milestones on issues and PRs.

For example, when creating an issue or a PR, make sure it has one or more
component labels (Traffic Ops, Traffic Monitor, etc) on it at a minimum. If
you are unable to attach labels, reach out to a "committer". They can add
labels for you.

Also, if you plan to work on an issue, make sure you assign it to yourself
(or put a comment that you are working on it) and select the appropriate
milestone. (again, ask a "committer" if you can't assign the milestone)

By using labels and milestones, I think it should be pretty easy to see
what is going in / went in to a particular release.

thoughts?

jeremy

Re: Delivery Service Properties

[Jeremy Mitchell]

I've also created an issue (
https://github.com/apache/incubator-trafficcontrol/issues/1543) and
subsequent PR (https://github.com/apache/incubator-trafficcontrol/pull/1548)
to address the different requirements of each type of DS.

On Wed, Nov 15, 2017 at 3:10 PM, Jeremy Mitchell <mitchell...@gmail.com>
wrote:

> after thinking about this a bit  more, I think I will just leave dscp,
> regional_geo_blocking and routing_name as NOT NULL in the database. it's
> probably easier to work around those requirements than removing the
> constraint and having to deal with the fallout of that.
>
> jeremy
>
> On Wed, Nov 15, 2017 at 2:02 PM, Jeremy Mitchell <mitchell...@gmail.com>
> wrote:
>
>> I can buy that. I'll leave the NOT NULL on routing_name alone. And yes, I
>> agree, ANY_MAP seems to be quite an oddball.
>>
>> On Wed, Nov 15, 2017 at 1:10 PM, Rawlin Peters <rawlin.pet...@gmail.com>
>> wrote:
>>
>>> I'm not a big fan of the ANY_MAP type DeliveryService. This is from
>>> the documentation (basically the only information about this type):
>>>
>>> """
>>> ANY_MAP is not known to Traffic Router. For this deliveryservice, the
>>> “Raw remap text” field in the input form will be used as the remap
>>> line on the cache.
>>> """
>>>
>>> That statement is false because if an ANY_MAP DS is set to active, it
>>> will make it into the CRConfig and be treated as an HTTP DS by the
>>> Traffic Router [1]. But when it's inactive, it's "Raw remap line"
>>> still gets deployed to its assigned caches remap.config [2]. I think
>>> the only problem it was trying to solve was adding special lines to
>>> remap.config for certain sets of caches, so I believe any TR-specific
>>> fields of a DS don't apply to the ANY_MAP type, i.e.:
>>>
>>> - active (because it should always be false yet still ends up in
>>> remap.config?)
>>> - ccr_dns_ttl
>>> - geo_limit
>>> - geo_limit_contries
>>> - geo_provider
>>> - geolimit_redirect_url
>>> - regional_geo_blocking
>>> - tr_request_headers
>>>
>>> For the routing_name column, if we remove the NOT NULL constraint,
>>> then we still have the problem of interpreting NULL as some string,
>>> and it's much easier to keep it NOT NULL and make sure a default gets
>>> inserted in the API layer rather than sprinkle an "if undefined use
>>> 'blah'" guard all over the codebase. So, if we removed the NOT NULL
>>> constraint, I think we'd still have to keep enforcing that constraint
>>> in the API layer so that no null values end up in the routing_name
>>> column.
>>>
>>> - Rawlin
>>>
>>> [1] https://github.com/apache/incubator-trafficcontrol/issues/1121
>>> [2] https://github.com/apache/incubator-trafficcontrol/issues/905
>>>
>>>
>>> On Wed, Nov 15, 2017 at 11:07 AM, Jeremy Mitchell <mitchell...@gmail.com>
>>> wrote:
>>> > I agree with you 100%, rob butts but i'd rather not tackle that big of
>>> a
>>> > database refactor at the moment. :)
>>> >
>>> > In the short term, I'd like to remove the NOT NULL database constraints
>>> > from the columns that don't pertain to ALL delivery services. Sadly,
>>> > overloading the ds table removed the ability to perform validation at
>>> the
>>> > database level which I think is actually pretty important but I guess
>>> we
>>> > can use the API/UI level validation for the time being.
>>> >
>>> > Jeremy
>>> >
>>> >
>>> >
>>> > On Wed, Nov 15, 2017 at 11:00 AM, Robert Butts <
>>> robert.o.bu...@gmail.com>
>>> > wrote:
>>> >
>>> >> If the field only applies to some DS types, it should be in its own
>>> table.
>>> >> Then it can be NOT NULL in that table, ideally with a constraint
>>> requiring
>>> >> the DS to be a type it applies to. You wanted to reduce the columns
>>> in the
>>> >> `deliveryservice` table, right? Here's your chance :)
>>> >>
>>> >> On Wed, Nov 15, 2017 at 10:56 AM, Jeremy Mitchell <
>>> mitchell...@apache.org>
>>> >> wrote:
>>> >>
>>> >> > From what I can tell, there are 4 flavors of delivery services:
>>> >> >
>>> >> > 1. DNS*
>>> >> > 2. HTTP*
>>> >&

Delivery Service Properties

[Jeremy Mitchell]

>From what I can tell, there are 4 flavors of delivery services:

1. DNS*
2. HTTP*
3. STEERING*
4. ANY_MAP

And the properties associated with each flavor vary so I put together a
spreadsheet to show which properties pertain to each flavor:

https://docs.google.com/spreadsheets/d/1rb4WzP1FicumyU1z6o_wasxD0ih1HoSH3bT-IA1GSlw/edit?usp=sharing

First things first, anything look wrong there? Have I misrepresented /
misunderstood anything?

Things that jump out at me:

1. dscp CANNOT be null yet it only applies to 2/4 of the delivery service
types (DNS* and HTTP*)
2. regional_geo_blocking CANNOT be null yet it only applies to 2/4 of the
delivery service types (HTTP* and ANY_MAP)
3. routing_name CANNOT be null yet it only applies to 3/4 of the delivery
service types (DNS*, HTTP* and STEERING*)
4. What's up with ssl_key_version? Is this even used? I don't see it in the
TO UI or the TP UI.

Suggestions:

1. we remove the NOT NULL constraint from dscp, regional_geo_blocking and
routing_name since they don't apply to ALL delivery services and rather we
enforce those fields in the API
2. we get rid of the ssl_key_version column unless someone knows if it's
used and how

Thoughts? Concerns?

Jeremy

Re: Traffic Ops API Semantic Versioning

[Jeremy Mitchell]

Actually, we haven't broken (on purpose) the API in a long time. Let's make
sure we're clear on what "breaking" changes are NOT.

They are NOT:


   - Adding new endpoints to the API
   - Adding new attributes to the response of an existing endpoint

^^ In my experience, this tends to be the bulk of our API changes which
just warrants a minor upgrade...

And I hope we're not changing the datatype of a minor field 13 times :)

On Thu, Oct 19, 2017 at 3:50 PM, Chris Lemmons <alfic...@gmail.com> wrote:

> I thought about that. But I'd like to reserve the major version of TC
> for actually major things. Making the API control the TC version
> number feels like the tail wagging the dog. Most other projects don't
> do it that way, either.
>
> It might not be so bad, except that we make breaking changes on a
> fairly regular basis. It would feel weird revving TC to version 14
> simply because we changed the datatype of a minor field for the 13th
> time.
>
> And likewise, sometimes we don't change the API. At the moment, we're
> pretty API-focused. But focuses change and we might have whole
> releases focused on UI or integrations. It would be strange to rev the
> API when it didn't change. The point of Semantic Versioning is to
> communicate changes to the API in a way that is consistent, easy to
> apply, and grokkable by humans and computers alike.
>
> I think I'm still -1 on locking the API version to the TC version.
>
> On Thu, Oct 19, 2017 at 3:39 PM, Robert Butts <robert.o.bu...@gmail.com>
> wrote:
> > @alficles What if we do both Semantic Versioning, and tying the API
> version
> > to the TC version? So, we would have to increase the TC major version
> with
> > any breaking API changes (which may inconveniently be minor to the rest
> of
> > TC).
> >
> > But, that has the big advantage of letting us programmatically tie the TC
> > version to every client. So, with a small upfront cost writing the code
> to
> > read the `/VERSION` file into the client binaries when the RPM is
> > built--for example, in Go with ```-ldflags "-X lib.Version=`cat
> > ../VERSION`"``, and then concatenating that string into the endpoint
> > request---we could synchronise all our versions, and essentially not have
> > to think about versions, not have to manually update clients, not have to
> > worry about what endpoint was added in what version.
> >
> > That alleviates a huge percentage of the brain and keyboard work we
> > currently have to do surrounding versioning.
> >
> >> Our API version should have 3 numbers, though: major, minor, patch
> >
> > -1 on putting the patch in the URI, it's verbose and unnecessary, bug
> fixes
> > don't make things incompatible and are unlikely to cause confusion.
> > +1 on putting the patch in a header, e.g. `X-API-Version: 1.3.deadbeef`.
> > Header makes it easy to debug, in the rare event it's necessary, without
> > cluttering the URI or Body.
> >
> >
> > On Thu, Oct 19, 2017 at 3:14 PM, Chris Lemmons <alfic...@gmail.com>
> wrote:
> >
> >> Heh, I agree on the arguing about API versions, but I just want to
> >> hand the task to Semantic Versioning, so we can just ask some simple
> >> questions like "did we break backward compatibility" and "did we add
> >> something" and figure out what number to put on the route. And we
> >> avoid arguments entirely. (We have to get used to the idea of revving
> >> the top version, though, because we break compatibility a lot.)
> >>
> >> On Thu, Oct 19, 2017 at 3:01 PM, Jeremy Mitchell <mitchell...@gmail.com
> >
> >> wrote:
> >> > In reality, we really did break the API when we went from mysql (TC
> 1.8)
> >> to
> >> > postgres (TC 2.0) and strings became ints, for example, so the api
> should
> >> > really be 2.x anyhow imo.
> >> >
> >> > Anyhow, the real reason i like syncing the api version to the TC
> version
> >> is
> >> > for simplicity and we don't have to argue all day about api versions.
> :)
> >> >
> >> >
> >> >
> >> >
> >> >
> >> > On Thu, Oct 19, 2017 at 2:54 PM, Chris Lemmons <alfic...@gmail.com>
> >> wrote:
> >> >
> >> >> Interesting idea. I think, though, it's better to keep API versions
> >> >> separate from TC versions. Many versions of TC won't rev the API at
> >> >> all. I'd prefer to go all in on Semantic Versioning.
> >> >>
> >> >> I think it could work like this. For a 

Re: Traffic Ops API Semantic Versioning

[Jeremy Mitchell]

In reality, we really did break the API when we went from mysql (TC 1.8) to
postgres (TC 2.0) and strings became ints, for example, so the api should
really be 2.x anyhow imo.

Anyhow, the real reason i like syncing the api version to the TC version is
for simplicity and we don't have to argue all day about api versions. :)





On Thu, Oct 19, 2017 at 2:54 PM, Chris Lemmons <alfic...@gmail.com> wrote:

> Interesting idea. I think, though, it's better to keep API versions
> separate from TC versions. Many versions of TC won't rev the API at
> all. I'd prefer to go all in on Semantic Versioning.
>
> I think it could work like this. For a given route, use the route
> defined for the most recent version not later than the requested
> version. So, if the quux endpoint is introduced at 1.2, and changed at
> 1.4, these requests would be served by these routes:
>
> GET /api/1.1/quux → 404
> GET /api/1.2/quux → Served by 1.2.
> GET /api/1.3/quux → Served by 1.2.
> GET /api/1.4/quux → Served by 1.4.
> GET /api/1.5/quux → Served by 1.4.
>
> The advantage of this is that it's relatively easy to implement in the
> router, though there are admittedly some challenges on the Perl side
> of things. If we want to be clever, we can allow a client to elide
> later values and just assume they meant the latest. But we don't need
> to start out clever.
>
> Our API version should have 3 numbers, though: major, minor, patch. We
> should expose these versions in a new /version endpoint that will
> allow clients to self-configure and detect new versions.
>
> As an alternative to strict semantic versioning, we could consider the
> first two numbers to be the "major" number for Semantic Versioning
> purposes. I think this hurts discoverability in the community, but it
> matches our current practice, for the most part. I think this would be
> a reasonable compromise, if people aren't comfortable increasing the
> first number every time we break compatibility in a revision.
>
> Lastly, we should recognize that we may need to retire API versions.
> Eventually, we may make changes to the DB that mean the information
> provided in a prior version simply does not exist. To that end, we
> should provide a /minversion (or similar) endpoint that returns the
> lowest API version supported by that server. Queries below that value
> cannot be guaranteed to succeed. (Or would it be better to cut it off
> an say that no queries for previous versions are permitted?)
>
> I'm +1 on Semantic Versioning. I'm a lukewarm +1 on a variant that
> uses the first two numbers as "major" and the last as "minor". I'm -1
> on locking it to the version of TC. I'm +1 on providing /version and
> /minversion endpoints to allow clients to auto-detect incompatibility.
>
> On Thu, Oct 19, 2017 at 12:54 PM, Jeremy Mitchell <mitchell...@gmail.com>
> wrote:
> > With the Golang API rewrite, we were not planning to break any APIs but
> > rather simply port them to Golang thus we did not see the need to rev the
> > API version from 1.2. However, maybe this is good opportunity to get our
> > API version inline with the TC version.
> >
> > So, my thought is that our Golang API's look like this:
> >
> > GET /api/v2.2/foos <-- this would return foos served by golang because
> > we've ported this endpoint
> > GET /api/v2.2/bars <-- this would return a 404 served by golang because
> > we've have NOT yet ported this endpoint
> >
> > and our perl apis are still acessible in 1.2
> >
> > GET /api/1.2/foos <-- this would return foos served by perl
> > GET /api/1.2/bars <-- this would return bars served by perl
> >
> > This has 3 benefits:
> >
> > 1. by revving the version, it signals the community that something has
> > actually changed in our API and in this case, the change may simply be
> the
> > fact that we now have nifty golang implemented APIs that you might want
> to
> > explore.
> > 2. it provides the ability to stay on the perl apis by specifying 1.2 if
> > that is what you want to do. maybe you are not much of a risk taker...
> > 3. it does actually provide license to break the new golang apis if
> needed
> > (to make them better) because the major version has incremented. it's
> nice
> > to have that option if needed.
> >
> > Also, going forward, I propose the TO API version follows in lockstep
> with
> > the TC version...which means the minor version will keep
> incrementing...GET
> > /api/v2.2/foos...GET /api/v2.3/foos..which means functionality can be
> added
> > to the API "in a backwards-compatible manner"
> >
> > Remember, at some point 

Re: Traffic Ops API Semantic Versioning

[Jeremy Mitchell]

Yes, when TC goes from 2.2 to 2.3 to 2.4, the TO API would follow those
versions. But since those are minor version upgrades, we can only "add
functionality [to the API] in a backwards-compatible manner" per your semantic
version definition above.

If we introduce API breaking changes, the API would go to 3.0, which would
pull up the version of TC to 3.0. (personally, i don't envision these types
of API changes for a while).



On Thu, Oct 19, 2017 at 2:12 PM, Eric Friedrich (efriedri) <
efrie...@cisco.com> wrote:

> If we want our TC version to match our API version, it looks like we would
> rev the version every TC minor release, even though it may not contain
> breaking changes?
>
>
>
> > On Oct 19, 2017, at 2:54 PM, Jeremy Mitchell <mitchell...@gmail.com>
> wrote:
> >
> > With the Golang API rewrite, we were not planning to break any APIs but
> > rather simply port them to Golang thus we did not see the need to rev the
> > API version from 1.2. However, maybe this is good opportunity to get our
> > API version inline with the TC version.
> >
> > So, my thought is that our Golang API's look like this:
> >
> > GET /api/v2.2/foos <-- this would return foos served by golang because
> > we've ported this endpoint
> > GET /api/v2.2/bars <-- this would return a 404 served by golang because
> > we've have NOT yet ported this endpoint
> >
> > and our perl apis are still acessible in 1.2
> >
> > GET /api/1.2/foos <-- this would return foos served by perl
> > GET /api/1.2/bars <-- this would return bars served by perl
> >
> > This has 3 benefits:
> >
> > 1. by revving the version, it signals the community that something has
> > actually changed in our API and in this case, the change may simply be
> the
> > fact that we now have nifty golang implemented APIs that you might want
> to
> > explore.
> > 2. it provides the ability to stay on the perl apis by specifying 1.2 if
> > that is what you want to do. maybe you are not much of a risk taker...
> > 3. it does actually provide license to break the new golang apis if
> needed
> > (to make them better) because the major version has incremented. it's
> nice
> > to have that option if needed.
> >
> > Also, going forward, I propose the TO API version follows in lockstep
> with
> > the TC version...which means the minor version will keep
> incrementing...GET
> > /api/v2.2/foos...GET /api/v2.3/foos..which means functionality can be
> added
> > to the API "in a backwards-compatible manner"
> >
> > Remember, at some point TO will only be the TO API so why wouldn't this
> > component follow the versioning that the other components do? Just my
> > thoughts.
> >
> > Jeremy
> >
> > On Tue, Oct 17, 2017 at 12:02 PM, Robert Butts <robert.o.bu...@gmail.com
> >
> > wrote:
> >
> >> I'm fully +1 on Semantic Versioning. We discussed it briefly on the
> list a
> >> long time ago, but we haven't really been doing it.
> >>
> >> That said, versioning requires a lot of code/work that simply doesn't
> exist
> >> today. That's the reason we haven't been doing it properly.
> >>
> >> The Go Traffic Ops has Semantic Versioning built-in to the Routing, but
> >> Perl support is close to nil. Perl currently has an easy way to say
> >> "include all the routes in this version number", But there's no way to
> say
> >> "this route is 1.2 only, and this route is 1.3 only" -- we'd have to
> >> duplicate TrafficOpsRoutes.pm, with only a few lines changed, and
> likewise
> >> duplicate changed functions. It needs a framework. Or we could just wait
> >> until Perl goes away.
> >>
> >> That said, Go, especially the client, doesn't completely support Minor
> >> Versions like it should, either. Consider adding a new field to Delivery
> >> Services. Per Semantic Versioning, that field MUST not be returned by
> the
> >> old version `GET`, and MUST not be set if passed by a `POST`. The Go
> server
> >> supports that in the Routing, via different endpoints, but there's no
> >> Struct framework or pattern. And the client completely doesn't support
> it.
> >> In Go, we probably need a set of anonymously-nested structs, `type
> >> DeliveryServices12 struct { Foo int }; type DeliveryServices13 struct
> >> {DeliveryServices12; Bar string}`. But that simply doesn't exist today,
> and
> >> will take development time to write.
> >>
> >> Option 2: Absolute Versioning. Instead of Se

Re: Traffic Ops API Semantic Versioning

[Jeremy Mitchell]

With the Golang API rewrite, we were not planning to break any APIs but
rather simply port them to Golang thus we did not see the need to rev the
API version from 1.2. However, maybe this is good opportunity to get our
API version inline with the TC version.

So, my thought is that our Golang API's look like this:

GET /api/v2.2/foos <-- this would return foos served by golang because
we've ported this endpoint
GET /api/v2.2/bars <-- this would return a 404 served by golang because
we've have NOT yet ported this endpoint

and our perl apis are still acessible in 1.2

GET /api/1.2/foos <-- this would return foos served by perl
GET /api/1.2/bars <-- this would return bars served by perl

This has 3 benefits:

1. by revving the version, it signals the community that something has
actually changed in our API and in this case, the change may simply be the
fact that we now have nifty golang implemented APIs that you might want to
explore.
2. it provides the ability to stay on the perl apis by specifying 1.2 if
that is what you want to do. maybe you are not much of a risk taker...
3. it does actually provide license to break the new golang apis if needed
(to make them better) because the major version has incremented. it's nice
to have that option if needed.

Also, going forward, I propose the TO API version follows in lockstep with
the TC version...which means the minor version will keep incrementing...GET
/api/v2.2/foos...GET /api/v2.3/foos..which means functionality can be added
to the API "in a backwards-compatible manner"

Remember, at some point TO will only be the TO API so why wouldn't this
component follow the versioning that the other components do? Just my
thoughts.

Jeremy

On Tue, Oct 17, 2017 at 12:02 PM, Robert Butts 
wrote:

> I'm fully +1 on Semantic Versioning. We discussed it briefly on the list a
> long time ago, but we haven't really been doing it.
>
> That said, versioning requires a lot of code/work that simply doesn't exist
> today. That's the reason we haven't been doing it properly.
>
> The Go Traffic Ops has Semantic Versioning built-in to the Routing, but
> Perl support is close to nil. Perl currently has an easy way to say
> "include all the routes in this version number", But there's no way to say
> "this route is 1.2 only, and this route is 1.3 only" -- we'd have to
> duplicate TrafficOpsRoutes.pm, with only a few lines changed, and likewise
> duplicate changed functions. It needs a framework. Or we could just wait
> until Perl goes away.
>
> That said, Go, especially the client, doesn't completely support Minor
> Versions like it should, either. Consider adding a new field to Delivery
> Services. Per Semantic Versioning, that field MUST not be returned by the
> old version `GET`, and MUST not be set if passed by a `POST`. The Go server
> supports that in the Routing, via different endpoints, but there's no
> Struct framework or pattern. And the client completely doesn't support it.
> In Go, we probably need a set of anonymously-nested structs, `type
> DeliveryServices12 struct { Foo int }; type DeliveryServices13 struct
> {DeliveryServices12; Bar string}`. But that simply doesn't exist today, and
> will take development time to write.
>
> Option 2: Absolute Versioning. Instead of Semantic Versioning, we could
> have a single version, and break compatibility with each new version. So,
> all new features (breaking or not) would go in a new version, and all
> clients must check the version, and refuse to operate on a different
> version. So you'd be required to have a client version matching the server
> version; users are not allowed to talk to new servers with old clients.
>
> Option 3: No Versioning. We get rid of the version number. Endpoints are at
> `/api/foo`. Over the last few years, we have _repeatedly_ broken the API
> for the same version. A 1.2 client from three years ago will fail
> catastrophically if connected to a Traffic Ops serving `/api/1.2` today. In
> practice, as we've been developing, we have no version, the version number
> is meaningless and confusing. If we're going to continue breaking
> compatibility without updating the version number, we should get rid of it.
> Then at least the version itself won't be confusing and painful.
>
> What's the consensus here? Does everyone agree with Semantic Versioning? Do
> we want to commit to requiring it? Is there a consensus? Or should we take
> a vote, whether to require Semantic Versioning, Absolute Versioning, or No
> Version?
>
>
> On Thu, Oct 12, 2017 at 7:39 AM, Dave Neuman  wrote:
>
> > Traffic ops currently does not handle versioning very well.  I think we
> do
> > support 1.1 and 1.2 versions of the API, but I think there are only a few
> > (maybe asns and deliveryservices) that are actually different.
> > Versioning is something we look to improve as we move to the golang
> version
> > of the API.
> >
> > On Thu, Oct 12, 2017 at 6:50 AM, Eric Friedrich (efriedri) <

Re: Traffic Ops Rewrite Golang Dependency - sqlx

[Jeremy Mitchell]

@dewayne - you said "performance was +/-5% depending on the cloud resources
that were active". How many milliseconds difference (average) are we
talking about for ONE call to /api/1.2/servers (using sqlx and not using
sqlx)? I'm assuming it is so small that we can rule out performance as a
factor when considering sqlx vs. no sqlx.

As someone who has to write a lot of the API code, I'm a big fan of
anything that increases developer productivity and as someone that has to
review a lot of API PR's, I'm a big fan of anything that improves
readability (and decreases the amount of lines of code I have to review).
Productivity + readability == my idea of a good time.

I'm confused. Why the fear of using dependencies? Do we plan to reinvent
the wheel for everything in this Golang API rewrite (routing, logging,
authentication, etc, etc) for fear that a 3rd-party library will stop being
maintained? If it were me, I would have installed Gorilla Mux (or something
similar) a long time ago, so we could get down to business of writing API
endpoints and not have to deal in the minutia (and all the inevitable bug
fixing) that comes with home-growing an API framework. Also, remember that
homegrown frameworks have zero resources on the internet. Say what you will
about Mojolicious but at least you could do some Googling to figure out how
to use it.

+1 on sqlx

jeremy

On Tue, Sep 12, 2017 at 8:37 PM, Chris Lemmons  wrote:

> I'm also -1 on sqlx.
>
> That said, the "single line" in the `sql` version is 674 characters long. I
> think we can agree that it's one heck of a line.
>
> We're focusing on the Scan call vs. the ScanStruct call, which I think is
> the right place to look. Here's what `sql` requires:
> - One identifier per column in the sql statement, ordered correctly, in the
> Scan call.
>
> Here's what `sqlx` requires:
> - One identifier per column in the sql statement, stored in the struct tag.
>
> In exchange for moving the identifiers from the struct tag to the scan
> call, you lose a bit of performance and add a third-party dependency.
> `sqlx` is moderately stable and moderately active, but both of those things
> can change fairly quickly. Dependencies are great, when they're worth the
> cost.
>
> We'd also be pulling in a fair bit of sqlx for what amounts to a single
> function. That's somewhat excessive, I think. We don't need most of what it
> offers.
>
> The second question is how much will using straight `sql` increase
> maintenance, and likewise with `sqlx`. For dependencies, we need to keep
> them up-to-date with bugfixes and security patches. We're not experts in
> `sqlx`, so bugs in that library will be comparatively expensive to trace
> and locate, if they exist. For the `sql` approach, the primary risk is just
> that the columns wind up out of order. The unit tests as they stand will
> prevent this.
>
> If we're really trying to skip having to type all the member variables in
> order... there's an easier way to do that anyway. We've already got a
> function returning the column names via reflection, we could just as easily
> do the exact same thing with their pointers and pass that straight to Scan.
> Which is the "best" of both worlds: you don't have to type the structs out
> and you don't have to store them in the struct tag. That said... I can't
> strongly recommend that technique because it has the same (though
> technically lesser) performance penalty as `sqlx`, and it's still pretty
> ugly. But it's better than pulling in an entire library for the benefit of
> one fairly reasonably written function.
>
> I guess the bottom line is that I don't think listing out the columns is
> that problematic. And even if it is, there are better solutions to that
> problem than pulling in the full `sqlx` library.
>
>
> On Tue, Sep 12, 2017 at 7:52 PM, Robert Butts 
> wrote:
>
> > I am a pretty big -1 on sqlx. Those PRs are extremely deceptive.
> >
> > Those lines are entirely unnecessary.
> >
> > I have created an example PR at https://github.com/apache/incu
> > bator-trafficcontrol/pull/1165
> >
> > The relevant commits are
> > https://github.com/apache/incubator-trafficcontrol/pull/1165
> > /commits/6fc735d7f97eaaffbf08e8457b7ccb6bf14baca0
> > https://github.com/apache/incubator-trafficcontrol/pull/1165
> > /commits/6939ee1d401c571af139db53b018a5e53f80c02a#diff-219ca
> > ea1a282285fe1cc21e53bf9dafbL26
> >
> > As you can see, the only difference is that `rows.StructScan()`
> becomes `
> > rows.Scan(, , , , 
> > DomainName, , , , , ,
> 
> > IloIpGateway, , , , 
> > InterfaceMtu, , , ,
> ,
> > , , , , 
> > MgmtIpGateway, , , , 
> > PhysLocationId, , , , , 
> > RevalPending, , , ,
> ,
> > , , , , , 
> > XmppPasswd)`
> >
> > It is a one-line difference per endpoint, not 100 lines. (Plus column
> > annotations on every struct field for sqlx)
> >
> > That said, I agree the former is better for readability. The issue is the
> > maintenance cost, when-not-if sqlx stops 

Re: [VOTE] Bugtracking in Github Issues

[Jeremy Mitchell]

+1 but the fact that only committers can be assigned to issues is a big
problem in my opinion.

On Tue, Aug 29, 2017 at 7:16 PM, Eric Friedrich (efriedri) <
efrie...@cisco.com> wrote:

> Thanks Leif-
>   I’ll check with the Incubator gurus. We already pulled the trigger, so
> at this point its more about asking for forgiveness.
>
> —Eric
> On Aug 29, 2017, at 6:47 PM, Leif Hedstrom  @apache.org>> wrote:
>
>
> On Aug 28, 2017, at 10:38 AM, Eric Friedrich (efriedri) <
> efrie...@cisco.com> wrote:
>
> We currently use JIRA Issues to track all of the Traffic Control bugs.
>
> Now that we have write access to Github, we can move back to GH Issues for
> bug tracking.
>
> This will be a better workflow because its one fewer tool and account to
> have to interact with. This will hopefully lower the bar for new
> contributors to file bugs and engage with the product. We can also help use
> it (along with the milestones) to create more useful changelogs and release
> notes.
>
> A possible downside is that the Issues are maybe less flexible than JIRA
> in terms of permissions, workflow, fields, etc. However, we were using
> Issues before we entered the incubator and that was fine for us. Hopefully
> no one has developed an attachment for JIRA in the last year.
>
>
> Please Vote +1 to move to Github Issues or -1 to stay on Jira. I’ll assume
> a lazy consensus if there aren’t enough votes.
>
>
>
> I’m +1 on this, but did you verify with the Incubator powers that this is
> allowed? Another option is to defer this until you are down with
> incubation, and make the migration to a TLP include moving more of your
> workflow over to Github proper.
>
> — Leif
>
>

Global parameters and seeds.sql

[Jeremy Mitchell]

IMO there seems to be an inherent flaw with global parameters and that flaw
is exposed in via seeds.sql.

Let's use an example.

There is a global parameter called tm.toolname with a seeded value of
'Traffic Ops' (
https://github.com/apache/incubator-trafficcontrol/blob/master/traffic_ops/app/db/seeds.sql#L39
)

Obviously, the intention of this parameter is that you can change it to fit
your needs. Maybe you want to change it to "Traffic Mops".

However, when seeds.sql runs again (on upgrade for example), you will end
up with a duplicate tm.toolname parameter because the unique constraint on
the parameter table is applied to name + config_file + value.

For "global" parameters, the unique constraint should really be on name +
config_file because, really, why would you want more than 1 global
parameter?

So we have a table (parameters) where some parameters the unique constraint
should be name + config_file + value (which makes perfect sense if you
think about how parameters are used with profiles) and other parameters
(global) where the unique constraint should be name + config_file...

Anyhow, after saying all of thatI think the solution (or a solution) is
to modify seeds.sql to first run a query (select count) for global
parameters to ensure that they are not already there before inserting
them

I'll create a jira for that if there is no objections.

jeremy

Re: Adding support for per-DeliveryService routing names

[Jeremy Mitchell]

thb i'm not 100% sure how db defaults work but i think in the DS api
create/update since routing_name is going to be optional, you'll want to
see if it exists (and not "") in the request json and if so, use that value
to update the db else retrieve the default (from wherever you decide) and
use that value to update the db.

jeremy



On Tue, Aug 15, 2017 at 9:14 AM, Peters, Rawlin <rawlin_pet...@comcast.com>
wrote:

> How do we feel about setting a default for the column in the DB schema?
> The routing name can be any arbitrary hostname (without periods) after this
> support is added, and if someone doesn’t like the default we choose, they
> can always provide/update their own routing_name via the UI/API.
>
> --Rawlin
>
> On 8/15/17, 8:42 AM, "Robert Butts" <robert.o.bu...@gmail.com> wrote:
>
> +1
>
> A new requirement is a breaking change, needs to be a new major
> version, or
> a default value that doesn't break existing usage.
>
> On Tue, Aug 15, 2017 at 8:40 AM, Dewayne Richardson <dewr...@gmail.com
> >
> wrote:
>
> > This is a tough one because the obvious ways of breaking an API are
> "URL
> > format changes", "Request or Response format changes", etc.  But I
> think
> > the more obvious way to think about the API is, do the clients have
> to
> > change their code?  If the answer is yes, it's a breaking API change.
> >
> > So, really the only way around this is to "default" the new field's
> value
> > and make it optional, otherwise the API needs to rev.
> >
> > -Dew
> >
> > On Tue, Aug 15, 2017 at 8:09 AM, Jeremy Mitchell <
> mitchell...@gmail.com>
> > wrote:
> >
> > > I don't think you can add a new not null column to the ds table
> because
> > > that would break the DS api. For example, on DS create/update you
> are
> > > saying routing_name is now required, right? This is an API breaking
> > change
> > > that would require a new version of the API, no?
> > >
> > > Hence my suggestion for a default. But, yes, i forgot about the
> per CDN
> > > thing.
> > >
> > > Jeremy
> > >
> > > On Mon, Aug 14, 2017 at 5:25 PM, Peters, Rawlin <
> > rawlin_pet...@comcast.com
> > > >
> > > wrote:
> > >
> > > > Jeremy’s suggestion could work, but the param would probably be
> created
> > > in
> > > > a TR_PROFILE per-CDN. However, that still wouldn’t fix the
> visibility
> > > > problem. If a CDN isn’t using the default “tr” HTTP routing name,
> > > operators
> > > > would still need to know that there is a new profile parameter
> that
> > needs
> > > > updating post-upgrade but before a snap/queue. So either way
> there
> > needs
> > > to
> > > > be sufficient upgrade notes, but personally I still prefer
> keeping the
> > > > routing_name column non-null.
> > > >
> > > > That said, this is my current proposal for the DB migration
> which also
> > > > gets us past the upgrade issue:
> > > > 1. Add a routing_name column to the DeliveryService table.
> > > > 2. Update the routing_name for DNS Delivery Services to “edge”.
> > > > 3. Update the routing_name of non-DNS Delivery Services to the
> value
> > of a
> > > > temporary upgrade parameter associated with the Delivery
> Service’s CDN
> > > (if
> > > > the upgrade parameter doesn’t exist, the routing_names will
> remain
> > null).
> > > > 4. Update the remaining null routing_names to “tr”.
> > > > 5. Make the routing_name column non-null and add a non-empty
> > constraint.
> > > >
> > > > So these would be an operator’s pre-upgrade steps:
> > > > 1. Verify if a custom http.routing.name has been configured for
> > Traffic
> > > > Routers in their CDNs.
> > > > 2. If custom http.routing.name, do the following. Otherwise, no
> > > > pre-upgrade steps needed (for per-DS routing names at least):
> > > > a. create a parameter named “upgrade_http_routing_name” with
> the
> > > value
> > > > of the target CDN’s custom http.routing.name
> > > > b. associate this parameter to the TR_PROFILE belonging

Re: Adding support for per-DeliveryService routing names

[Jeremy Mitchell]

I don't think you can add a new not null column to the ds table because
that would break the DS api. For example, on DS create/update you are
saying routing_name is now required, right? This is an API breaking change
that would require a new version of the API, no?

Hence my suggestion for a default. But, yes, i forgot about the per CDN
thing.

Jeremy

On Mon, Aug 14, 2017 at 5:25 PM, Peters, Rawlin <rawlin_pet...@comcast.com>
wrote:

> Jeremy’s suggestion could work, but the param would probably be created in
> a TR_PROFILE per-CDN. However, that still wouldn’t fix the visibility
> problem. If a CDN isn’t using the default “tr” HTTP routing name, operators
> would still need to know that there is a new profile parameter that needs
> updating post-upgrade but before a snap/queue. So either way there needs to
> be sufficient upgrade notes, but personally I still prefer keeping the
> routing_name column non-null.
>
> That said, this is my current proposal for the DB migration which also
> gets us past the upgrade issue:
> 1. Add a routing_name column to the DeliveryService table.
> 2. Update the routing_name for DNS Delivery Services to “edge”.
> 3. Update the routing_name of non-DNS Delivery Services to the value of a
> temporary upgrade parameter associated with the Delivery Service’s CDN (if
> the upgrade parameter doesn’t exist, the routing_names will remain null).
> 4. Update the remaining null routing_names to “tr”.
> 5. Make the routing_name column non-null and add a non-empty constraint.
>
> So these would be an operator’s pre-upgrade steps:
> 1. Verify if a custom http.routing.name has been configured for Traffic
> Routers in their CDNs.
> 2. If custom http.routing.name, do the following. Otherwise, no
> pre-upgrade steps needed (for per-DS routing names at least):
> a. create a parameter named “upgrade_http_routing_name” with the value
> of the target CDN’s custom http.routing.name
> b. associate this parameter to the TR_PROFILE belonging to the target
> CDN
> c. repeat steps 2a and 2b for each CDN using a custom
> http.routing.name
>
> This would keep everything working the same post-upgrade as it did
> pre-upgrade, and from that point on you’d be able to change a Delivery
> Service’s routing name to any arbitrary hostname (without periods).
>
> --Rawlin
>
> On 8/14/17, 4:22 PM, "Dave Neuman" <neu...@apache.org> wrote:
>
> I don't think that solves the issue Rawlin was describing.  The issue
> that
> Rawlin was describing is that someone has already defined a different
> routing name in traffic_router/http.properties which is no longer
> going to
> be used after the upgrade.  The upgrade needs to somehow know about
> this
> other routing name and use that when it creates the database column and
> populates it with the pre-defined defaults (edge, tr).
>
> Also, creating a global param doesn't help those with more than 1 CDN.
>
> On Mon, Aug 14, 2017 at 4:09 PM, Jeremy Mitchell <
> mitchell...@gmail.com>
> wrote:
>
> > Adding a temp parameter would work but I worry that someone won't
> read the
> > upgrade documentation and forget to create this temporary parameter
> before
> > running the upgrade.
> >
> > Here's another option.
> >
> > Create 2 global TO parameters (http.routing.name and
> dns.routing.name
> > <http://http.routing.name/>) that default to tr and edge
> respectively and
> > make the ds.routing_name an optional field.
> >
> > in seeds.sql
> >
> > insert into parameter (name, config_file, value) values ('
> > http.routing.name',
> > 'global', 'tr') ON CONFLICT (name, config_file, value) DO NOTHING;
> > insert into parameter (name, config_file, value) values ('
> dns.routing.name
> > ',
> > 'global', 'edge') ON CONFLICT (name, config_file, value) DO NOTHING;
> >
> > in code (warning. ugly pseudo code to follow):
> >
> > function getRoutingName(ds) {
> > return ds.routing_name if not null
> > if (ds.type = HTTP) {
> > return parameter.http.routing.name
> > } else
> > return parameter.dns.routing.name
> > }
> > }
> >
> > Just my 2 cents.
> >
> > On Mon, Aug 14, 2017 at 10:55 AM, Dave Neuman <neu...@apache.org>
> wrote:
> >
> > > Good info Rawlin.
> > > My vote would be for a parameter to be used during the upgrade.
> We can
> > set
> > > a param called `upgrade_routing_name` or something similiar so
> that it is
>

Re: Adding support for per-DeliveryService routing names

[Jeremy Mitchell]

Adding a temp parameter would work but I worry that someone won't read the
upgrade documentation and forget to create this temporary parameter before
running the upgrade.

Here's another option.

Create 2 global TO parameters (http.routing.name and dns.routing.name
) that default to tr and edge respectively and
make the ds.routing_name an optional field.

in seeds.sql

insert into parameter (name, config_file, value) values ('http.routing.name',
'global', 'tr') ON CONFLICT (name, config_file, value) DO NOTHING;
insert into parameter (name, config_file, value) values ('dns.routing.name',
'global', 'edge') ON CONFLICT (name, config_file, value) DO NOTHING;

in code (warning. ugly pseudo code to follow):

function getRoutingName(ds) {
return ds.routing_name if not null
if (ds.type = HTTP) {
return parameter.http.routing.name
} else
return parameter.dns.routing.name
}
}

Just my 2 cents.

On Mon, Aug 14, 2017 at 10:55 AM, Dave Neuman  wrote:

> Good info Rawlin.
> My vote would be for a parameter to be used during the upgrade.  We can set
> a param called `upgrade_routing_name` or something similiar so that it is
> obvious that it is a param used for upgrade only.  We should also document
> that A) the param needs to be set before upgrade and B) TR will now ignore
> the setting in the config file.  Ideally we would remove the param from the
> default config and the need for the param in the code.
>
> On Wed, Aug 9, 2017 at 4:28 PM, Peters, Rawlin 
> wrote:
>
> > Hey all,
> >
> > I’ve dug through this a bit more, and defaulting a new
> > DeliveryService.routing_name
> > column to ‘tr’ for HTTP delivery services presents an upgrade issue if a
> > CDN has
> > chosen to use a custom “http.routing.name” parameter for the Traffic
> > Routers
> > in that CDN (by editing the http.properties files of the Traffic
> Routers).
> >
> > For instance, if “http.routing.name” has been set to “ccr”, the new
> > routing name
> > “tr” will break all of the clients using the old “ccr” delivery service
> > URL.
> >
> > Basically we need to provide a one-time upgrade step to allow CDNs using
> a
> > custom
> > “http.routing.name” to default the new routing_name column to that value
> > for
> > existing HTTP delivery services. What would be the best way to do this?
> > Some options
> > might be:
> > 1. Add a profile parameter to the TR_PROFILE for that CDN. On upgrade,
> > read that
> > parameter and use it to update the routing_name for existing HTTP
> > delivery services.
> > After upgrade, you can safely remove the profile parameter.
> > 2. Let the upgrade automatically default the routing_name to ‘tr’ or
> > ‘edge’. After
> > upgrading, manually update each HTTP delivery service to use the
> > current
> > “http.routing.name” in use (we could provide an API endpoint to
> “bulk
> > update” the
> > routing names for all HTTP delivery services in a CDN).
> >
> > Note this is not an exhaustive list, this is a just a couple options that
> > have come up in
> > discussion so far. Feel free to add any more ideas to this list.
> >
> > After the upgrade has been completed, the “http.routing.name” parameter
> > in the
> > Traffic Router’s “http.properties” file will be ignored (same with the “
> > dns.routing.name”
> > parameter in “dns.properties” which I’m not sure can even be changed
> > safely today).
> >
> > Thoughts?
> >
> > --Rawlin
> >
> > On 8/4/17, 10:19 AM, "Peters, Rawlin"  wrote:
> >
> > @Dave @JvD
> >
> > Thanks for the feedback. I think I can get on board with defaulting
> > the DS columns to ‘edge’ and ‘tr’. I was thinking the CDN columns might
> be
> > useful if the user just wants to set it CDN-wide and not individually on
> > each DS, but I guess if we default it as part of the upgrade migration,
> we
> > should also provide an API endpoint to set the routing names on all DSes
> in
> > a CDN to a single value, thus still providing a “per-CDN” option. Would a
> > “bulk” update also be useful, in case a user wants to update a handful of
> > DSes to the same routing names at once?
> >
> > @JvD Re: TR_PROFILE vs. DS_PROFILE
> > The default would come from a TR_PROFILE linked to the CDN, and the
> > override would come from a DS_PROFILE linked to that specific DS. I
> looked
> > into those options to cover all my bases, but I think adding columns to
> at
> > least the DS table and not touching profiles at all is the better option.
> >
> > -Rawlin
> >
> > On 8/4/17, 8:04 AM, "Jan van Doorn"  wrote:
> >
> > Agree with Dave on
> >
> > [*DN] we should default the database column to "edge" for DNS and
> > "tr" for*
> > *http.  Then we don't have to do the null check.*
> >
> > If we do that, we can make the columns mandatory, and it makes
> > sense
> > they're not in the DS_PROFILE. Also makes it so we don't have to
> > have a CDN
> 

Re: Data patches during upgrade

[Jeremy Mitchell]

This seems to make sense to me but honestly, I'd probably defer to Dewayne.

In theory, it would be nice if migrations only included "structural"
changes (new tables, columns, changing column types or not  null, etc) and
seeds focused on the "base" (or the minimum required) static data required
of TO (types, statuses, roles, etc) and then yea, putting data fixing or
data massaging as the last step makes sense to me. But you know what they
say about theory...

+1

Jeremy

On Wed, Jun 7, 2017 at 8:41 AM, Gelinas, Derek 
wrote:

> I'm adding a feature to traffic ops that creates a new column in
> steering_target called type, that is populated with type ids from the type
> table.  Using admin.pl upgrade, the column is created in migrations, and
> the two types for this table are populated by seeds.sql.  None of this is
> out of the ordinary.  Unfortunately I also need to populate the type column
> based on data that isn't in there until after seeds.sql is run, so I can't
> place this into the migration.  Seeds.sql needs to run after the migration
> due to any structural changes that happen there.
>
> Dewayne and I have discussed this a bit this morning, and we're thinking
> the best solution might be a third step, run after seeds.sql, called
> patches.sql.  This would be specifically for data fixes like in this use
> case.  The order would be as follows:
>
> migration - structure
> seeds - static data
> patches - data fixes
>
> Thoughts?
>
> Derek

Re: LDAP Access

[Jeremy Mitchell]

> @mitchell852 Actual PostgreSQL users. So, Traffic Ops users would _be_
PostgreSQL users. There wouldn't be a single "trafficops" Postgres user,
every TO user would have their own user in Postgres itself.

^^ Sounds like we need a Postgres DBA for that :) Plus, I think that ship
has sailed when the roles/capabilities model was agreed upon and is
currently in the works...

Just to clarify for others, currently, when you login via LDAP and no user
is found in the tm_user table with the same username, we assign the
"read-only" role to the "current user". Rob doesn't think (and he's
probably right) that the "read-only" role is restrictive enough for these
LDAP only users. There are lots of GET routes that "read-only" users can
access that contain "sensitive" info... (depending on your definition of
sensitive)

Anyhow, with this PR -
https://github.com/apache/incubator-trafficcontrol/pull/544 - the concept
of "capabilities" was introducedroles can now have capabilitiesso
in theory we could create a role called "Graph Viewer" or something which
maps to the cdn-graph-read capability that maps to certain api
endpoints...and then the "Graph Viewer" role would be automatically
assigned to LDAP only users..just an example.

^^ however the role/capabilities thing only applies to the API (not the
current TO UI) and was to be enforced by the API gateway

In my opinion, trying to do what's best for the TO UI and the TO API at the
same time is very difficult, thus, my push to deprecate the TO UI and the
entire UI namespace of endpoints, in favor of the TO API and the API
namespace... TO API + API Gateway + Roles/Capabilities gives us more
granularity

Jeremy





On Thu, Jun 1, 2017 at 9:39 AM, Jeff Elsloo  wrote:

> We use LDAP all the time. It's optional of course, but in our
> deployment nobody should be using local accounts unless they're not in
> LDAP for some reason (external users, portal users, etc).
> Application/API accounts could go either way, but users of the TO UI
> should use LDAP whenever possible to avoid having to manage multiple
> accounts/passwords.
>
> Basic enterprise operations best practices dictate centralized user
> management, and most enterprises do so using some sort of LDAP based
> directory (Active Directory, OpenLDAP, etc). I'm a hard -1 on moving
> away from this model. If anything we need to make our LDAP
> implementation more flexible.
> --
> Thanks,
> Jeff
>
>
> On Thu, Jun 1, 2017 at 9:10 AM, Dewayne Richardson 
> wrote:
> > I have a question in a similar vein, how often do we really use LDAP?  My
> > understanding is we created LDAP access to allow external users in to see
> > our TO Graphs.  Now that graphs are in Graphana is the need for LDAP
> still
> > needed?  If we require anyone using TO or the TO API to be in the
> database
> > it would alleviate this LDAP security issue entirely.
> >
> >  I also wonder if we shouldn't try to leverage transitioning our user
> > management to Postgres.  Postgres has many options for authentication
> (as I
> > mentioned at the Summit), which would allow for more flexibility at TO
> > installations.
> >
> > -Dewayne
> >
> > On Wed, May 31, 2017 at 9:24 AM, Robert Butts 
> > wrote:
> >
> >> We have a PR https://github.com/apache/incubator-trafficcontrol/pull/
> 627
> >> to
> >> change Traffic Ops to only allow LDAP users _not_ in the Traffic Ops
> >> database to view non-sensitive information, like graphs and total CDN
> >> bandwidth.
> >>
> >> To be clear, users will still be able to authenticate with LDAP, as
> long as
> >> their user name is in the database. This only prevents access for LDAP
> >> users whose name is not in the database.
> >>
> >> If you have LDAP-only users who need access, you can simply add their
> user
> >> name to the Traffic Ops database to allow continued access. They don't
> even
> >> need a password, simply inserting the username is sufficient.
> >>
> >> LDAP is a security risk, especially for large organizations. Allowing
> all
> >> non-CDN personnel in the organization full information access, even
> >> read-only, means an attacker has only to compromise a single account in
> the
> >> organization, and they can see the full list of CDN server IPs and
> FDQNs,
> >> as well as the specific ATS and CentOS versions, in order to take
> advantage
> >> of known exploits against those versions.
> >>
> >> Does anyone have any issues with that? Is anyone using LDAP without
> >> usernames in the database, who needs continued access? We just want to
> make
> >> sure we're not breaking anyone before we merge this, and figure out a
> >> solution if we are. Thanks,
> >>
>

Re: LDAP Access

[Jeremy Mitchell]

> I also wonder if we shouldn't try to leverage transitioning our user
management to Postgres.

I don't understand what that means. We do use Postgres for user
management...there is a tm_user table in Postgres and a user has a role
(which will soon have capabilities). That is how users are managed and it
would be a pretty big shift to move away from that...

I think I'm fine with Rob's PR -
https://github.com/apache/incubator-trafficcontrol/pull/627

which will basically break everything if you are depending on LDAP only.
For example, if my LDAP username/pass is jeremy/password but there is no
user in the tm_user table with username=jeremy, then my authentication will
work but I will not have a pleasant experience in Traffic Ops as pretty
much every route will now result in a 404.

..but the workaround is to create a user with username=jeremy and assign me
a role..

and that's rob's intent..if you are ldap only, nothing really
works...(except a few routes)

Jeremy

On Thu, Jun 1, 2017 at 9:10 AM, Dewayne Richardson 
wrote:

> I have a question in a similar vein, how often do we really use LDAP?  My
> understanding is we created LDAP access to allow external users in to see
> our TO Graphs.  Now that graphs are in Graphana is the need for LDAP still
> needed?  If we require anyone using TO or the TO API to be in the database
> it would alleviate this LDAP security issue entirely.
>
>  I also wonder if we shouldn't try to leverage transitioning our user
> management to Postgres.  Postgres has many options for authentication (as I
> mentioned at the Summit), which would allow for more flexibility at TO
> installations.
>
> -Dewayne
>
> On Wed, May 31, 2017 at 9:24 AM, Robert Butts 
> wrote:
>
> > We have a PR https://github.com/apache/incubator-trafficcontrol/pull/627
> > to
> > change Traffic Ops to only allow LDAP users _not_ in the Traffic Ops
> > database to view non-sensitive information, like graphs and total CDN
> > bandwidth.
> >
> > To be clear, users will still be able to authenticate with LDAP, as long
> as
> > their user name is in the database. This only prevents access for LDAP
> > users whose name is not in the database.
> >
> > If you have LDAP-only users who need access, you can simply add their
> user
> > name to the Traffic Ops database to allow continued access. They don't
> even
> > need a password, simply inserting the username is sufficient.
> >
> > LDAP is a security risk, especially for large organizations. Allowing all
> > non-CDN personnel in the organization full information access, even
> > read-only, means an attacker has only to compromise a single account in
> the
> > organization, and they can see the full list of CDN server IPs and FDQNs,
> > as well as the specific ATS and CentOS versions, in order to take
> advantage
> > of known exploits against those versions.
> >
> > Does anyone have any issues with that? Is anyone using LDAP without
> > usernames in the database, who needs continued access? We just want to
> make
> > sure we're not breaking anyone before we merge this, and figure out a
> > solution if we are. Thanks,
> >
>

Re: Duplicate TO API routes

[Jeremy Mitchell]

@Eric_Friedrich - any concerns from you regarding removal of these
duplicate routes? Here they are to summarize:

remove GET /api/$version/deliveryservices/list in favor of GET
/api/$version/deliveryservices
remove GET /api/$version/deliveryservices/:id/get in favor of GET
/api/$version/deliveryservices/:id
remove POST /api/$version/deliveryservices/create in favor of POST
/api/$version/deliveryservices
remove PUT /api/$version/deliveryservices/:id/update in favor of PUT
/api/$version/deliveryservices/:id
remove GET /api/$version/cachegroups/list in favor of GET
/api/$version/cachegroups
remove POST /api/$version/cachegroups/create in favor of POST
/api/$version/cachegroups
remove PUT /api/$version/cachegroups/:id/update in favor of PUT
/api/$version/cachegroups/:id
remove DELETE /api/$version/cachegroups/:id/delete in favor of DELETE
/api/$version/cachegroups/:id
remove POST /api/$version/servers/create in favor of POST
/api/$version/servers
remove PUT /api/$version/servers/:id/update in favor of PUT
/api/$version/servers/:id

On Fri, Apr 28, 2017 at 10:32 AM, Dewayne Richardson <dewr...@gmail.com>
wrote:

> And as long as the "duplicate" routes point to the same Perl module, I'm ok
> as well
>
> On Tue, Apr 18, 2017 at 4:27 PM, Dave Neuman <neu...@apache.org> wrote:
>
> > I assume you did a check of the client to make sure it wasn't using those
> > routes?
> > Other than that, no concerns here.
> >
> > On Tue, Apr 18, 2017 at 12:37 PM, Jeremy Mitchell <
> mitchell...@apache.org>
> > wrote:
> >
> > > As we move towards a new authorization model on the TO API which
> includes
> > > roles, capabilities and tenancy, it's important to remove any cruft
> from
> > > the API to ease this new implementation. I've put some effort into
> > > organizing the TO routes file (
> > > https://github.com/apache/incubator-trafficcontrol/blob/
> > > master/traffic_ops/app/lib/TrafficOpsRoutes.pm#L373)
> > > but I'd also like to remove any duplicate routes.
> > >
> > > We have a few duplicates mainly in the area of deliveryservice,
> servers,
> > > and cachegroups. This jira outlines the duplicates and suggests which
> > > routes to remove - https://issues.apache.org/jira/browse/TC-82
> > >
> > > If these routes are being used, they would not be removed until 2.1
> > giving
> > > everyone time to use the standard ones.
> > >
> > > Concerns?
> > >
> > > Jeremy
> > >
> >
>

Re: [VOTE] Move Traffic Control to full GitHub

[Jeremy Mitchell]

+1

On Thu, May 18, 2017 at 3:35 PM, Dave Neuman  wrote:

> +1, thanks for putting the vote up Jan
>
> On Thu, May 18, 2017 at 4:57 PM, Steve Malenfant 
> wrote:
>
> > +1
> >
>

Re: API GW route configuration

[Jeremy Mitchell]

Here was the image I was trying to attach:

https://cwiki.apache.org/confluence/display/TC/API+Gateway

Jeremy

On Thu, May 11, 2017 at 2:14 PM, Amir Yeshurun <am...@qwilt.com> wrote:

> Hi Jeremy,
> Note that attachments seems to be stripped off on this list and the image
> is unavailable.
>
> Your assumptions are correct. We need to figure out the easiest topology
> for UI routes to bypass the GW. Please reattach the picture so we can get
> more specific.
>
> Thanks
> /amiry
>
>
>
> On Thu, May 11, 2017, 20:06 Jeremy Mitchell <mitchell...@gmail.com> wrote:
>
> > What is of utmost importance to me is the ability to ease into this. We
> > have a TO UI right now that needs to be unaffected by the API gateway in
> my
> > opinion. Granted the old UI might go away at some point but until that
> time
> > it needs to function as-is.
> >
> > To me, the simplest approach is to key off request URL. anything that
> > starts with /api gets api gateway treatment, the rest passes on
> > thru...Here's a fancy picture to communicate what I envision...
> >
> > [image: Inline image 1]
> >
> > I'm assuming all requests (endpoints) go thru the api gateway but maybe
> > i'm wrong in that assumption. Anyhow, i guess my point is the UI should
> > continue to work with the mojo cookie and "api" calls should use the jwt
> > token...however, the UI also uses api endpoints so not sure how that
> would
> > work...
> >
> > If it's too difficult for the api gateway to support UI and API routes,
> we
> > could always wait until the new UI (which leverages the API) is
> complete...
> >
> > Jeremy
> >
> > On Thu, May 11, 2017 at 10:23 AM, Chris Lemmons <alfic...@gmail.com>
> > wrote:
> >
> >> > invalidate ALL tokens by changing the token signing key
> >>
> >> Interesting idea. That does mean that the signing key has to be
> retrieved
> >> every time from the authentication authority, or it'd be subject to the
> >> exact same set of attacks. But a nearly-constant rarely changing key
> could
> >> be communicated very efficiently, I suspect. And if the authentication
> >> system is a web API, it can even use Modified-Since to 304 99% of the
> time
> >> for maximum efficiency.
> >>
> >> It does have the downside that key-invalidation events are fairly
> >> significant. You'd need to invalidate the keys whenever someone's access
> >> was reduced or removed. As the number of accounts in the system
> increases,
> >> that might not wind up being as infrequent as one might hope. It's easy
> to
> >> implement, though.
> >>
> >> On Thu, May 11, 2017 at 10:12 AM Jeremy Mitchell <mitchell...@gmail.com
> >
> >> wrote:
> >>
> >> > Regarding the TTL on the JWT token. a 5 minute TTL seems silly. What's
> >> the
> >> > point? Unless we get into refresh tokens but that sounds like
> >> oauth...blah.
> >> >
> >> > What about this and maybe i'm oversimplifying. the TTL on the jwt
> token
> >> is
> >> > 24 hours. If we become aware that a token has been compromised,
> >> invalidate
> >> > ALL tokens by changing the token signing key. maybe this is a good
> idea
> >> or
> >> > maybe this is a terrible idea. I have no idea. just a thought..
> >> >
> >> > jeremy
> >> >
> >> > On Wed, May 10, 2017 at 12:23 PM, Chris Lemmons <alfic...@gmail.com>
> >> > wrote:
> >> >
> >> > > Responding to a few people:
> >> > >
> >> > > > Often times every auth action must be accompanied by DB writes for
> >> > audit
> >> > > logs or callback functions.
> >> > >
> >> > > True. But a) if logging is too expensive it should probably be made
> >> > cheaper
> >> > > and b) the answer to "audits are too expensive" probably isn't "lets
> >> just
> >> > > do less authentication". If the audit log is genuinely the
> >> bottle-neck,
> >> > it
> >> > > would still be better to re-auth without the audit log.
> >> > >
> >> > > > The API gateway can poll for the latest list of tokens at a
> regular
> >> > > interval
> >> > >
> >> > > Yeah, datastore replication for local performance is great. Though
> if
> >> you
> >> > > can reasonably query

Re: API GW route configuration

[Jeremy Mitchell]

What is of utmost importance to me is the ability to ease into this. We
have a TO UI right now that needs to be unaffected by the API gateway in my
opinion. Granted the old UI might go away at some point but until that time
it needs to function as-is.

To me, the simplest approach is to key off request URL. anything that
starts with /api gets api gateway treatment, the rest passes on
thru...Here's a fancy picture to communicate what I envision...

[image: Inline image 1]

I'm assuming all requests (endpoints) go thru the api gateway but maybe i'm
wrong in that assumption. Anyhow, i guess my point is the UI should
continue to work with the mojo cookie and "api" calls should use the jwt
token...however, the UI also uses api endpoints so not sure how that would
work...

If it's too difficult for the api gateway to support UI and API routes, we
could always wait until the new UI (which leverages the API) is complete...

Jeremy

On Thu, May 11, 2017 at 10:23 AM, Chris Lemmons <alfic...@gmail.com> wrote:

> > invalidate ALL tokens by changing the token signing key
>
> Interesting idea. That does mean that the signing key has to be retrieved
> every time from the authentication authority, or it'd be subject to the
> exact same set of attacks. But a nearly-constant rarely changing key could
> be communicated very efficiently, I suspect. And if the authentication
> system is a web API, it can even use Modified-Since to 304 99% of the time
> for maximum efficiency.
>
> It does have the downside that key-invalidation events are fairly
> significant. You'd need to invalidate the keys whenever someone's access
> was reduced or removed. As the number of accounts in the system increases,
> that might not wind up being as infrequent as one might hope. It's easy to
> implement, though.
>
> On Thu, May 11, 2017 at 10:12 AM Jeremy Mitchell <mitchell...@gmail.com>
> wrote:
>
> > Regarding the TTL on the JWT token. a 5 minute TTL seems silly. What's
> the
> > point? Unless we get into refresh tokens but that sounds like
> oauth...blah.
> >
> > What about this and maybe i'm oversimplifying. the TTL on the jwt token
> is
> > 24 hours. If we become aware that a token has been compromised,
> invalidate
> > ALL tokens by changing the token signing key. maybe this is a good idea
> or
> > maybe this is a terrible idea. I have no idea. just a thought..
> >
> > jeremy
> >
> > On Wed, May 10, 2017 at 12:23 PM, Chris Lemmons <alfic...@gmail.com>
> > wrote:
> >
> > > Responding to a few people:
> > >
> > > > Often times every auth action must be accompanied by DB writes for
> > audit
> > > logs or callback functions.
> > >
> > > True. But a) if logging is too expensive it should probably be made
> > cheaper
> > > and b) the answer to "audits are too expensive" probably isn't "lets
> just
> > > do less authentication". If the audit log is genuinely the bottle-neck,
> > it
> > > would still be better to re-auth without the audit log.
> > >
> > > > The API gateway can poll for the latest list of tokens at a regular
> > > interval
> > >
> > > Yeah, datastore replication for local performance is great. Though if
> you
> > > can reasonably query for a list of all valid tokens every second, it's
> > > probably cheaper to to just query for the token you need every time you
> > > need it. If there are massive batches of queries that are coming
> through,
> > > it's probably not unreasonable to choose not to re-validate a token
> > that's
> > > been validated in the last second.
> > >
> > > > Regarding maliciously delayed message or such - I don't fully
> > understand
> > > the
> > > point; if an attacker has such capabilities she can simply
> prevent/delay
> > > devop users from updating the auth database itself thus enabling the
> > > attack.
> > >
> > > In a typical attack, an attacker might gain control of a box on the
> local
> > > network, but not necessarily the Gateway, Traffic Ops, or Auth Server.
> > > Those are probably better hardened. But lots of networks have a squishy
> > > test box that everyone forgot was there or something. The bad guy wants
> > to
> > > use the CDN to DOS someone, or redirect traffic to somewhere malicious,
> > or
> > > just cause mayhem. The longer he can keep control, the better for him.
> > >
> > > So this attacker uses the local box to sniff the token off the network.
> > If
> > > the communication with the Gateway is encrypted, he

Re: API GW route configuration

[Jeremy Mitchell]

Regarding the TTL on the JWT token. a 5 minute TTL seems silly. What's the
point? Unless we get into refresh tokens but that sounds like oauth...blah.

What about this and maybe i'm oversimplifying. the TTL on the jwt token is
24 hours. If we become aware that a token has been compromised, invalidate
ALL tokens by changing the token signing key. maybe this is a good idea or
maybe this is a terrible idea. I have no idea. just a thought..

jeremy

On Wed, May 10, 2017 at 12:23 PM, Chris Lemmons  wrote:

> Responding to a few people:
>
> > Often times every auth action must be accompanied by DB writes for audit
> logs or callback functions.
>
> True. But a) if logging is too expensive it should probably be made cheaper
> and b) the answer to "audits are too expensive" probably isn't "lets just
> do less authentication". If the audit log is genuinely the bottle-neck, it
> would still be better to re-auth without the audit log.
>
> > The API gateway can poll for the latest list of tokens at a regular
> interval
>
> Yeah, datastore replication for local performance is great. Though if you
> can reasonably query for a list of all valid tokens every second, it's
> probably cheaper to to just query for the token you need every time you
> need it. If there are massive batches of queries that are coming through,
> it's probably not unreasonable to choose not to re-validate a token that's
> been validated in the last second.
>
> > Regarding maliciously delayed message or such - I don't fully understand
> the
> point; if an attacker has such capabilities she can simply prevent/delay
> devop users from updating the auth database itself thus enabling the
> attack.
>
> In a typical attack, an attacker might gain control of a box on the local
> network, but not necessarily the Gateway, Traffic Ops, or Auth Server.
> Those are probably better hardened. But lots of networks have a squishy
> test box that everyone forgot was there or something. The bad guy wants to
> use the CDN to DOS someone, or redirect traffic to somewhere malicious, or
> just cause mayhem. The longer he can keep control, the better for him.
>
> So this attacker uses the local box to sniff the token off the network. If
> the communication with the Gateway is encrypted, he might have to do some
> ARP poisoning or something else to trick a host into talking to the local
> box instead. (Properly implemented TLS also migates this angle.) He knows
> that as soon as he starts his nefarious deed, alarms are going to go off,
> so he also uses this local box to DOS the Auth Server. It's a lot easier to
> take a box down from the outside than to actually gain control.
>
> If the Gateway "fails open" when it can't contact the Auth server, the
> attacker remains in control. If it "fails closed", the attacker has to
> actually compromise the auth server (which is harder) to remain in control.
>
> > Do we block all API calls if the auth service is temporarily down (being
> upgraded, container restarting, etc…)?
>
> Yes, I think we have to. Authentication is integral to reliable operation.
>
> We've been talking in some fairly wild hypotheticals, though. Is there a
> specific auth service you're envisioning?
>
> On Wed, May 10, 2017 at 12:50 AM Shmulik Asafi  wrote:
>
> > Regarding the communication issue Chris raised - there is more than one
> > possible pattern to this, e.g.:
> >
> >- Blacklisted tokens can be communicated via a pub-sub mechanism
> >- The API gateway can poll for the latest list of tokens at a regular
> >interval (which can be very short ~1sec, much shorter than the time it
> >takes devops to detect and react to malign tokens)
> >
> > Regarding hitting the blacklist datastore - this only sounds similar to
> > hitting to auth database; but the simplicity of a blacklist function
> allows
> > you to employ more efficient datastores, e.g. Redis or just a hashmap in
> > the API gateway process memory.
> >
> > Regarding maliciously delayed message or such - I don't fully understand
> > the point; if an attacker has such capabilities she can simply
> > prevent/delay devop users from updating the auth database itself thus
> > enabling the attack.
> >
> >
> > On Wed, May 10, 2017 at 4:25 AM, Eric Friedrich (efriedri) <
> > efrie...@cisco.com> wrote:
> >
> > > Our current management wrapper around Traffic Control (called OMD
> > > Director, demo’d at last TC summit) uses a very similar approach to
> > > authentication.
> > >
> > > We have an auth service that issues a JWT. The JWT is then provided
> along
> > > with all API calls. A few comments on our practical experience:
> > >
> > > - I am a supported of validating tokens both in the API gateway and in
> > the
> > > service. We have several examples of services- Grafana for example,
> that
> > > require external authentication. Similarly, we have other services that
> > > need finer grained authentication than API Gateway policy can handle.
> > > 

Re: Removing 'internal' from TO API

[Jeremy Mitchell]

I'm not 100% familiar with how the API gateway will work but I always
assumed that it was a microservice that handled everything /api/*. For
example, if you made a request to traffic-ops.domain.com/foo.jpg, the api
gateway would not kick inbut if you made a request to
traffic-ops.domain.com/api/whatever, the api gateway would kick in and do
it's thing (check roles, capabilities, token, etc).

If that is true, it seems like we need to get these "internal" routes under
the api namespace.

We have 8 "internal" routes:

4 authenticated (you have to be logged in) steering routes that are locked
down by role (admin or steering):

$r->get("/internal/api/$version/steering")->over( authenticated => 1 )->to(
'Steering#index', namespace => 'API::DeliveryService' );
$r->get("/internal/api/$version/steering/:xml_id")->over( authenticated =>
1 )->to( 'Steering#index', namespace => 'API::DeliveryService' );
$r->post("/internal/api/$version/steering")->over( authenticated => 1
)->to( 'Steering#add', namespace => 'API::DeliveryService' );
$r->put("/internal/api/$version/steering/:xml_id")->over( authenticated =>
1 )->to( 'Steering#update', namespace => 'API::DeliveryService' );

1 authenticated federation route locked down by role (admin):

$r->get("/internal/api/$version/federations")->over( authenticated => 1
)->to( 'Federation#index', namespace => $namespace );

^^ imo those are all authenticated and locked down by role so there should
be no harm losing the "internal". (as long as any place is the code base
that uses these routes are adjusted)

1 unauthenticated (you don't have to be logged in) dnsseckeys route:

$r->get("/internal/api/$version/cdns/dnsseckeys/refresh")->to(
'Cdn#dnssec_keys_refresh', namespace => $namespace );

2 unauthenticated traffic stats routes:

$r->get("internal/api/$version/daily_summary")->to(
'CacheStats#daily_summary', namespace => $namespace );
$r->get("internal/api/$version/current_stats")->to(
'CacheStats#current_stats', namespace => $namespace );

At the very least, the first 5 routes should be "fixed" imo. Not sure what
to do about the other 3.

Jeremy




On Fri, Mar 17, 2017 at 5:40 AM, Amir Yeshurun <am...@qwilt.com> wrote:

> With the API GW, such duplications, or modifications can be defined in the
> GW, if required, instead of in TO
>
> On Thu, Mar 16, 2017, 5:52 PM Jan van Doorn <j...@knutsel.com> wrote:
>
> > We should also think about the API gateway future I think with that,
> we
> > don't need these special routes at all anymore, right Amir?
> >
> > Cheers,
> > JvD
> >
> >
> > On Wed, Mar 15, 2017 at 9:24 PM Dewayne Richardson <dewr...@gmail.com>
> > wrote:
> >
> > > I think we should do as Dave mentioned, assess and rename.
> > >
> > > > On Mar 15, 2017, at 2:18 PM, Jeremy Mitchell <mitchell...@gmail.com>
> > > wrote:
> > > >
> > > > I don't like duplicating routes either but I thought it would ease
> the
> > > > transition rather than just changing the route. So no code
> duplication,
> > > > just 2 routes that go to the same place:
> > > >
> > > > $r->get("/internal/api/$version/steering")->over( authenticated => 1
> > > )->to(
> > > > 'Steering#index', namespace => 'API::DeliveryService' );
> > > > $r->get("/api/$version/steering")->over( authenticated => 1 )->to(
> > > > 'Steering#index', namespace => 'API::DeliveryService' );
> > > >
> > > > And then we circle back and delete
> > > >
> > > > $r->get("/internal/api/$version/steering")->over( authenticated => 1
> > > )->to(
> > > > 'Steering#index', namespace => 'API::DeliveryService' );
> > > >
> > > > at some point.
> > > >
> > > > And yes, this internal namespace was introduced for comcast-specific
> > > > reasons that I believe no longer exist.
> > > >
> > > > Jeremy
> > > >
> > > >
> > > >
> > > > On Wed, Mar 15, 2017 at 2:13 PM, David Neuman
> > > > wrote:
> > > >
> > > > > At least a few of those (Steering, federations) were put in the
> > > "internal"
> > > > > namespace to work around Comcast specific issues. I don't know
> that I
> > > like
> > > > > the idea of duplicating routes, if anything we should see what is
> > > impacted
> > > >

Re: Access Control - Limiting Roles / Capabilities Tenant Admins can Assign to Users

[Jeremy Mitchell]

@Eric - I don't think we want to take the access control granularity below
the API level. That would make things pretty messy imo and I think this new
roles/capabilities model might be enough to digest as it is.

so basically, if you have a role that has a capability that maps to the
POST /api/version/users api endpoint (create user), then you can create
users. But of course, new users need a role. I think we should just
leverage what we have in place now - priv_level.

So, basically, if I have the ability to create users, I can only create
users with a role that has a priv_level <= my role's priv level.

I don't know if i want to add another hierarchy (role hierarchy)the
less hierarchies the better :)

Jeremy

On Thu, May 4, 2017 at 6:39 AM, Eric Friedrich (efriedri) <
efrie...@cisco.com> wrote:

> Could we further differentiate the user creation capabilities to:
> - Create CDN Admin user
> - Create CDN Ops user
> - Create CDN Viewer user
> - Create Tenant Admin user
> - Create Tenant Ops user
> - Create Tenant Viewer user
>
> Then only the CDN-Admin role would have the capability to create a cdn
> admin user. Would be good to see the capabilities assigned at a granularity
> below API endpoint in this case.
>
> As for creation of new roles, I like #2 and #3. Users should not be able
> to level-up anyone’s capabilities beyond their own. Further, capabilities
> are enforced by code, so we should not allow creation of new capabilities
> by API
>
> - - Eric
>
>
>
> On May 3, 2017, at 9:44 AM, Durfey, Ryan <ryan_dur...@comcast.com ryan_dur...@comcast.com>> wrote:
>
> Moving this active debate into the mailing list.
> -Jeremy makes a good point.  We need a method for making restricting roles
> and capabilities for lower tier staff that can create new users.  Jeremy
> has suggested a point system or a hierarchy.  I think either of these would
> work if applied correctly.   I am open to any approach that works.
>
> My thoughts:
> 1. We need to limit which users can build new roles from capabilities or
> new capabilities from APIs.  This could be limited to a master role like
> “CDN Admin”.  Otherwise other admins could circumvent the system by
> matching APIs to lower tier roles.
> 2. Another simple approach may be to only allow non-CDN Admins to assign
> roles to users which they have access.  Basically you can’t give anyone
> more rights than you have.
> 3. Perhaps with this approach we allow non-CDN Admins to build roles from
> existing capabilities to which they have access, but not create
> capabilities from APIs.  Then they can build new roles and assign any
> capabilities or roles to which they already have access.
>
>
>
> From: Jeremy Mitchell
>
> I like this model of a user has a role which has capabilities which map to
> API endpoints, however, there seems to be one flaw or at least one
> unaccounted for use case.
> Let's look at the roles listed above:
>
>   *   CDN-Admin
>   *   CDN-Ops
>   *   CDN-Viewer
>   *   Tenant-Admin
>   *   Tenant-Ops
>   *   Tenant-Viewer
>
> Jeremy is a CDN-Admin which has the user-create capability (among others)
> so he creates Bob, a Tenant-Admin. Being a Tenant-Admin, Bob also has
> user-create so he creates Sally and he can give her ANY role so he decides
> to give Sally the CDN-Admin rolewhoops, we don't want that...
> Bob should be limited to creating users with role=Tenant-Admin (his role),
> Tenant-Ops or Tenant-Viewer...but how do we correlate one role with
> another? Currently, we have "privilege level" attached to a role. So I
> guess we could use that like so:
>
>   *   CDN-Admin (100)
>   *   CDN-Ops (50)
>   *   CDN-Viewer (40)
>   *   Tenant-Admin (30)
>   *   Tenant-Ops (20)
>   *   Tenant-Viewer (10)
>
> Now, being a Tenant-Admin with the user-create capability, Bob can only
> create users where role.priv_level is 30 or below. I feel like this might
> be the easiest solution.
> Thoughts?
>
>
> ...
> Now, being a Tenant-Admin with the user-create capability, Bob can only
> create users where role.priv_level is 30 or below. I feel like this might
> be the easiest solution.
> Or...you could make roles hierarchical the way that tenants are
> hierarchical
> -CDN-Admin
> --CDN-Ops
> --CDN-Viewer
> --Tenant-Admin
> ---Tenant-Ops
> ---Tenant-Viewer
> And in this scenario, if you have the user-create capability you can
> create users with your role or a child of your role...
> Thoughts?
>
>
> Ryan Durfey
> Sr. Product Manager - CDN | Comcast Technology Solutions
> 1899 Wynkoop Ste. 550 | Denver, CO 8020
> M | 303-524-5099
> ryan_dur...@comcast.com<mailto:ryan_dur...@comcast.com>
> 24x7 CDN Support: 866-405-2993  or cdn_supp...@comcast.com cdn_supp...@comcast.com>
>
>
>

Re: Proposed changes to xml_id on a delivery service (from the API perspective)

[Jeremy Mitchell]

Great feedback. It sounds like people are OK with making xml_id immutable
on a delivery service... but not so much about making the host header regex
at position 0 immutable (the regex that requires this format -
.*\xml-id\..*).

Just to revisit:

You create a delivery service with xml_id=foo-bar and by default you get a
host header regex in this format at position 0 - .*\xml-id\..* so you end
up with URL=edge|tr.foo-bar.cdn.domain.com

Maybe you're happy with that and life is good.

Maybe you decide you want your URL to be edge|
tr.something-else.cdn.domain.com instead.so you need the ability to
edit the host header regex at position 0 to .*\something-else\..*

What about this? You can attach Static DNS entries to a delivery service.
Can Static DNS entries be used to accomplish this use case while leaving
.*\xml-id\..* alone? (Elsloo keyed me into that possibility)

Jeremy


On Fri, Apr 28, 2017 at 9:44 AM, Dave Neuman <neu...@apache.org> wrote:

> I don't think we should be forcing users into using their xml_id as the
> host_regex used to create the delivery URL.  I think we can offer to create
> it for them, but we should also let users have the ability to define what
> their regex needs to be when they create their delivery service.  We have
> instances in our environment where the host regex does not match the xml_id
> a delivery service and I don't think we should take that functionality
> away.
> You are correct, changing an xml_id or a host_regex could have unintended
> consequences in the system, but I agree with Ori that not letting users
> change it can cause other problems.  I think we should allow users to
> change it but maybe we need to figure out a way to only allow "super
> special advanced" users to do it.
>
> --Dave
>
> On Thu, Apr 27, 2017 at 6:40 AM, Ori Finkelman <o...@qwilt.com> wrote:
>
> > Hi Jeremy,
> >
> > I would like to refer to bullet 2 in your proposal.
> > Coupling between the xml_id and the host regexp, making the host regexp
> > immutable may create undesired limitations.
> > For example, if, for any reason, the content provider has to change the
> > host regexp, she will have to create a new DS. This limitation prevents
> > long term reports on DS volume, running long term analytics and other
> > operations that you may want to run over long periods of time.
> > In general, the host regexp is an attribute of the DS and therefore,
> unlike
> > the id which should be immutable, I think it should be mutable, like any
> > other attribute.
> >
> > A side issue is that we would like to suggest promoting TC-55
> > <https://issues.apache.org/jira/browse/TC-55>
> > <https://issues.apache.org/jira/browse/TC-55>in order to allow DS to be
> > matched by a combination of host regexp and path regexp. TC-55
> > <https://issues.apache.org/jira/browse/TC-55> requires it should be
> > allowed
> > to have the same host regexp for different DS, using the path regexp as
> > differentiator. This option has the benefit of using a single certificate
> > for multiple DSs, it also reduces the complexity of delegating traffic
> > between CDNs, like the use cases of CDNi and Open Caching.
> > Unless I misunderstand the proposal, binding the first host regexp with
> the
> > xml_id makes it impossible to have the same host for different DS, as
> > required by TC-55 <https://issues.apache.org/jira/browse/TC-55>.
> >
> > Thanks,
> > Ori
> >
> > On Thu, Apr 27, 2017 at 6:37 AM, Zhilin Huang (zhilhuan) <
> > zhilh...@cisco.com
> > > wrote:
> >
> > > Hi Jeremy,
> > >
> > > I like the idea that make xml_id and ds.regex at position 0 immutable.
> > > Actually we have suffered some issues on HTTPs delivery services, and I
> > had
> > > ever created issue TC-187 with PR #360.
> > >
> > > Besides ds.regex at position 0, the ds.regex at other position of type
> > > HOST_REGEXP is not working for HTTPs. I think the reason is SAN is no
> > > supported in currently implementation in “generate_ssl_keys”. Do we
> have
> > > any plan to support multiple subdomain for HTTPs?
> > >
> > > If we do plan to support SAN, I think we need also consider the
> > > modification for ds.regex at position other than 0.
> > >
> > >
> > > Thanks,
> > > Zhilin
> > >
> > >
> > >
> > > On 4/27/17, 1:41 AM, "Jeremy Mitchell" <mitchell...@apache.org> wrote:
> > >
> > > As we port functionality from the existing TO UI to the TO API, I
> am
> > > always
> > > looking f

Re: Duplicate TO API routes

[Jeremy Mitchell]

yep, not used by TO client, dave.

On Tue, Apr 18, 2017 at 4:27 PM, Dave Neuman <neu...@apache.org> wrote:

> I assume you did a check of the client to make sure it wasn't using those
> routes?
> Other than that, no concerns here.
>
> On Tue, Apr 18, 2017 at 12:37 PM, Jeremy Mitchell <mitchell...@apache.org>
> wrote:
>
> > As we move towards a new authorization model on the TO API which includes
> > roles, capabilities and tenancy, it's important to remove any cruft from
> > the API to ease this new implementation. I've put some effort into
> > organizing the TO routes file (
> > https://github.com/apache/incubator-trafficcontrol/blob/
> > master/traffic_ops/app/lib/TrafficOpsRoutes.pm#L373)
> > but I'd also like to remove any duplicate routes.
> >
> > We have a few duplicates mainly in the area of deliveryservice, servers,
> > and cachegroups. This jira outlines the duplicates and suggests which
> > routes to remove - https://issues.apache.org/jira/browse/TC-82
> >
> > If these routes are being used, they would not be removed until 2.1
> giving
> > everyone time to use the standard ones.
> >
> > Concerns?
> >
> > Jeremy
> >
>

Re: Traffic Ops database unique constraints

[Jeremy Mitchell]

I created a Jira - https://issues.apache.org/jira/browse/TC-222

I'll submit a PR shortly if there are no objections.

On Tue, Apr 4, 2017 at 2:18 PM, Jeremy Mitchell <mitchell...@gmail.com>
wrote:

> and i forgot one. i'd like to make parameter unique by
> name/config_file/value
>
> On Tue, Apr 4, 2017 at 1:55 PM, Jan van Doorn <j...@knutsel.com> wrote:
>
>> +1
>> > On Apr 4, 2017, at 1:17 PM, Jeremy Mitchell <mitchell...@apache.org>
>> wrote:
>> >
>> > If I add these database constraints, you can't create 2 statuses (for
>> > example) with the same name (which I think is probably the desired
>> > effect)...so I can change my seeded data to look like:
>> >
>> > insert into status (name, description) values ('OFFLINE', 'Server is
>> > Offline. Not active in any configuration.') ON CONFLICT (name) DO
>> NOTHING;
>> >
>> > and no more duplicate seeds...
>> >
>> > On Tue, Apr 4, 2017 at 1:12 PM, Jeremy Mitchell <mitchell...@apache.org
>> >
>> > wrote:
>> >
>> >> I have moved all Traffic Ops seed data into ONE place -
>> >> https://github.com/apache/incubator-trafficcontrol/blob/
>> >> master/traffic_ops/app/db/seeds.sql
>> >>
>> >> (there used to be some in seeds.sql and others in
>> >> traffic_ops/install/data/json)
>> >>
>> >> Anyhow, if you run seeds.sql multiple times (i.e. db/admin.pl
>> upgrade),
>> >> you'll end up with duplicate data as some tables don't have unique
>> >> constraints.
>> >>
>> >> I'd like to add unique constraints to the following database table /
>> >> columns:
>> >>
>> >> - role.name
>> >> - status.name
>> >> - type.name
>> >> - job_status.name
>> >>
>> >> In my opinion, these constraints should have been in there since day 1
>> but
>> >> if you have any objections, let me know.
>> >>
>> >> Thanks,
>> >>
>> >> Jeremy
>> >>
>> >>
>> >>
>>
>>
>

Re: 2.0 release?

[Jeremy Mitchell]

These all got fixed and backported to 2.0:

TC-203: Mojo doesn’t set cachable headers on public files”
TC-190: TTL type mismatch in CrConfig
TC-189: ssl_multicert.config too slow

So Jan and Dave just need to close the issues.

On Tue, Apr 4, 2017 at 12:22 PM, Jeffrey Martin 
wrote:

> Hi Eric,
> I was going to address the immediate Postinstall issues TC-185. I am way
> late on this. I created a fork yesterday, need to run a couple of tests and
> then I can push to this fork.
> Jeff Martin
>
>
> On Tue, Apr 4, 2017 at 1:59 PM, Eric Friedrich (efriedri) <
> efrie...@cisco.com> wrote:
>
> > We have some release blockers for 2.0. Specifically:
> >
> > TC-119: traffic_ops/client dependency license issue
> > We cannot ship with Category-X LGPL software, so I’m waiting for this
> > to be resolved before cutting a release branch
> >  "TC-185 post install doesn’t run due to missing perl module”
> > We shouldn’t ship a release in which the install process is broken in
> > this way.
> >*There’s no assignee yet for this, any volunteers?*
> >
> > I think if we can get those two taken care of we can cut an RC0 later
> this
> > week.
> >
> > Major bugs we will ship with (unless someone objects):
> > TC-203: Mojo doesn’t set cachable headers on public files”
> > TC-190: TTL type mismatch in CrConfig
> > TC-189: ssl_multicert.config too slow
> >
> > —Eric
> >
> >
> >
> >
> >
> > > On Apr 4, 2017, at 1:13 PM, Dave Neuman  wrote:
> > >
> > > Good question.  I would also like to see us try to get some release
> > > candidates out for 2.0.  I am pretty sure the actual install and
> > > postinstall need work.  There are also a couple of issue that are still
> > > assigned to 2.0 and unresolved:
> > > https://issues.apache.org/jira/browse/TC/fixforversion/
> > 12338562/?selectedTab=com.atlassian.jira.jira-projects-
> > plugin:version-summary-panel
> > > .
> > >
> > > On Tue, Apr 4, 2017 at 11:05 AM, Jan van Doorn 
> wrote:
> > >
> > >> When are we planning to release 2.0? We at Comcast are running what we
> > >> call 2.0…. So we are +1, I am pretty sure.
> > >>
> > >> Eric: are you waiting for something? Which cats need herding?
> > >>
> > >> Rgds,
> > >> JvD
> > >>
> > >>
> >
> >
>

Re: Traffic Ops database unique constraints

[Jeremy Mitchell]

If I add these database constraints, you can't create 2 statuses (for
example) with the same name (which I think is probably the desired
effect)...so I can change my seeded data to look like:

insert into status (name, description) values ('OFFLINE', 'Server is
Offline. Not active in any configuration.') ON CONFLICT (name) DO NOTHING;

and no more duplicate seeds...

On Tue, Apr 4, 2017 at 1:12 PM, Jeremy Mitchell <mitchell...@apache.org>
wrote:

> I have moved all Traffic Ops seed data into ONE place -
> https://github.com/apache/incubator-trafficcontrol/blob/
> master/traffic_ops/app/db/seeds.sql
>
> (there used to be some in seeds.sql and others in
> traffic_ops/install/data/json)
>
> Anyhow, if you run seeds.sql multiple times (i.e. db/admin.pl upgrade),
> you'll end up with duplicate data as some tables don't have unique
> constraints.
>
> I'd like to add unique constraints to the following database table /
> columns:
>
> - role.name
> - status.name
> - type.name
> - job_status.name
>
> In my opinion, these constraints should have been in there since day 1 but
> if you have any objections, let me know.
>
> Thanks,
>
> Jeremy
>
>
>

Traffic Ops database unique constraints

[Jeremy Mitchell]

I have moved all Traffic Ops seed data into ONE place -
https://github.com/apache/incubator-trafficcontrol/blob/master/traffic_ops/app/db/seeds.sql

(there used to be some in seeds.sql and others in
traffic_ops/install/data/json)

Anyhow, if you run seeds.sql multiple times (i.e. db/admin.pl upgrade),
you'll end up with duplicate data as some tables don't have unique
constraints.

I'd like to add unique constraints to the following database table /
columns:

- role.name
- status.name
- type.name
- job_status.name

In my opinion, these constraints should have been in there since day 1 but
if you have any objections, let me know.

Thanks,

Jeremy

Re: Changing Traffic Portal default page

[Jeremy Mitchell]

Unfortunately, there isn't a way currently to create custom html pages and
override the existing html pages of the traffic portal. There is, however,
a way to override styles -
https://github.com/apache/incubator-trafficcontrol/blob/master/traffic_portal/app/src/assets/css/custom.css

but...that won't help you.

You could, however, rewrite home.tpl.html to your liking but you would not
be able to commit it to the repository as it would most likely be specific
to your company.

Jeremy

On Mon, Mar 20, 2017 at 6:23 AM, Oren Shemesh  wrote:

> Hi Traffic Portal developers,
>
> I would like to make a (Hopefully small) change in the Portal code, so that
> the default page served when accessing the portal, will be some html page I
> am writing.
>
> Naturally, the page will be stored below /opt/traffic_portal/public/.
>
> Is there a way to change the Portal application so that when browsing to
> http://portal.something.com, the user will be served with such a
> preconfigured page, instead of the default login screen ?
>
> Regards, Oren.
>
> --
>
> *Oren Shemesh*
> Qwilt | Work: +972-72-2221637| Mobile: +972-50-2281168 | or...@qwilt.com
> 
>

Re: Removing 'internal' from TO API

[Jeremy Mitchell]

I don't like duplicating routes either but I thought it would ease the
transition rather than just changing the route. So no code duplication,
just 2 routes that go to the same place:

$r->get("/internal/api/$version/steering")->over( authenticated => 1 )->to(
'Steering#index', namespace => 'API::DeliveryService' );
$r->get("/api/$version/steering")->over( authenticated => 1 )->to(
'Steering#index', namespace => 'API::DeliveryService' );

And then we circle back and delete

$r->get("/internal/api/$version/steering")->over( authenticated => 1 )->to(
'Steering#index', namespace => 'API::DeliveryService' );

at some point.

And yes, this internal namespace was introduced for comcast-specific
reasons that I believe no longer exist.

Jeremy



On Wed, Mar 15, 2017 at 2:13 PM, David Neuman <david.neuma...@gmail.com>
wrote:

> At least a few of those (Steering, federations) were put in the "internal"
> namespace to work around Comcast specific issues.  I don't know that I like
> the idea of duplicating routes, if anything we should see what is impacted
> by moving them out of the internal namespace.
>
> On Wed, Mar 15, 2017 at 1:30 PM, Jeremy Mitchell <mitchell...@apache.org>
> wrote:
>
> > Currently, we have a number of API routes scoped as "internal". Here are
> a
> > few examples:
> >
> > https://github.com/apache/incubator-trafficcontrol/blob/
> > master/traffic_ops/app/lib/TrafficOpsRoutes.pm#L516
> >
> > I believe this is going to make it more difficult as we try to implement
> > more granular roles / capabilities coupled with tenancy.
> >
> > So I'm proposing that we create a duplicate non-internal route like this,
> > for example:
> >
> > $r->get("/api/$version/steering")->over( authenticated => 1 )->to(
> > 'Steering#index', namespace => 'API::DeliveryService' );
> >
> > that way we can slowly move away from the "internal" routes and
> eventually
> > deprecate them.
> >
> > I think with our upcoming more robust role / tenancy model, there is no
> > longer a need for "internal".
> >
> > Thoughts?
> >
> > Jeremy
> >
>

Re: Github "Backport" Label

[Jeremy Mitchell]

Actually, I'm a bit surprised you were even able to create the 'backport'
label - https://github.com/apache/incubator-trafficcontrol/labels

how did you do that? please tell. :)

On Mon, Mar 13, 2017 at 7:59 PM, Jeremy Mitchell <mitchell...@gmail.com>
wrote:

> I didn't know we could create labels and apply them. If so, I think this
> would be fantastic...but like Dave, I don't see how to do it. :(
>
> On Mon, Mar 13, 2017 at 7:57 AM, Phil Sorber <sor...@apache.org> wrote:
>
>> You can also search on the branch base. Something like: "is:pr is:open
>> base:2.0.x"
>>
>> On Mon, Mar 13, 2017 at 7:55 AM Dave Neuman <neu...@apache.org> wrote:
>>
>> > I see the label, but I don't see a way to add the label to existing PRs.
>> >
>> > On Sun, Mar 12, 2017 at 8:38 PM, Eric Friedrich (efriedri) <
>> > efrie...@cisco.com> wrote:
>> >
>> > > I created a new label in Github called “backport”.
>> > >
>> > > Please tag any PRs with this label to make them easier to find.
>> > >
>> > > —Eric
>> > >
>> > >
>> >
>>
>
>

Re: Github "Backport" Label

[Jeremy Mitchell]

I didn't know we could create labels and apply them. If so, I think this
would be fantastic...but like Dave, I don't see how to do it. :(

On Mon, Mar 13, 2017 at 7:57 AM, Phil Sorber  wrote:

> You can also search on the branch base. Something like: "is:pr is:open
> base:2.0.x"
>
> On Mon, Mar 13, 2017 at 7:55 AM Dave Neuman  wrote:
>
> > I see the label, but I don't see a way to add the label to existing PRs.
> >
> > On Sun, Mar 12, 2017 at 8:38 PM, Eric Friedrich (efriedri) <
> > efrie...@cisco.com> wrote:
> >
> > > I created a new label in Github called “backport”.
> > >
> > > Please tag any PRs with this label to make them easier to find.
> > >
> > > —Eric
> > >
> > >
> >
>

Re: Access Control Model

[Jeremy Mitchell]

Going forward with this approach of roles (what you can do) and tenancy
(what scope you are restricted to) should tighten up the holes and make it
much easier to see what a role can / cannot do as roles will be mapped to
api endpoints. The way roles are setup today, it is hard to discern what
each role can actually do so I'm not surprised the read-only role is a
little too loose.

To summarize a bit (Nir or Naama, correct me if I'm wrong).

For phase 1:

A user will have one role. A role has N capabilities. A capability has N
apis.
A user will have one or no tenant.
A deliveryservice will have one or no tenant.
A cdn will have one or no tenant.

Example:

Jeremy has the DS Manager role that has the ds-read and the ds-write
capabilities. ds-read is GET /ds and GET /ds/:id. ds-write is POST /ds and
PUT /ds/:id
Jeremy has the Foo tenant

This means Jeremy can access these api endpoints:
GET /ds (get all ds's)
GET /ds/:id (get one ds)
POST /ds (create a ds)
PUT /ds/:id (update a ds)

but...only for ds's that belong to the Foo tenant.

Bob has the Server Manager role that has the server-read and the
server-write capabilities. server-read is GET /servers and GET
/servers/:id. server-write is POST /servers and PUT /servers/:id
Bob has the Bar tenant

This means Bob can access these api endpoints:
GET /servers (get all servers)
GET /servers/:id (get one server)
POST /servers (create a server)
PUT /servers/:id (update a server)

but...only for the servers that belong to the cdn that belong to the Bar
tenant.

also, you'll see that by making tenant optional on user, ds and cdn, it
will allow everyone to ease into this tenancy model. if a ds has no
tenant...then anyone can see it...

Jeremy

On Fri, Feb 24, 2017 at 2:37 PM, David Neuman <david.neuma...@gmail.com>
wrote:

> From what I understand, LDAP is not the issue, it is the fact that we give
> everyone read-only by default and read-only users can get sensitive
> information.  I think we want to keep LDAP but not allow read-only?
>
> On Fri, Feb 24, 2017 at 2:31 PM, Eric Friedrich (efriedri) <
> efrie...@cisco.com> wrote:
>
> > Do you have a suggested replacement for SSO?
> >
> > —Eric
> >
> > > On Feb 24, 2017, at 4:26 PM, Dewayne Richardson <dewr...@gmail.com>
> > wrote:
> > >
> > > re: Dave on LDAP
> > >
> > > LDAP should be avoided as we are now finding that it's not a
> preferrable
> > > way of logging in based upon Security Concerns.  If needed for some
> > reason
> > > down the road we should consider it as a separate Micro Service that
> can
> > be
> > > "bolted" on.
> > >
> > > -Dew
> > >
> > > On Wed, Feb 22, 2017 at 10:33 AM, Jeremy Mitchell <
> mitchell...@gmail.com
> > >
> > > wrote:
> > >
> > >> +1 on Dave's question. ldap is an edge case that we probably have to
> > >> account for. so with this new approach, 2 things are checked - your
> role
> > >> (what you can do) and your tenancy (scope of what you can do).
> > >>
> > >> first one - role:
> > >>
> > >> we'll need to think how ldap authenticated users (that don't have an
> > actual
> > >> row in the users table) are automatically assigned a role. Maybe the
> > roles
> > >> table is seeded with 2 roles like this: (remember, a role is a
> grouping
> > of
> > >> "capabilities" (or apis) )
> > >>
> > >> admin
> > >> - GET /api/ds
> > >> - GET /api/ds/:id
> > >> - PUT /api/ds/:id
> > >> - POST /api/ds
> > >> - DELETE  /api/ds/:id
> > >> - everything other api that TO supports
> > >>
> > >> read-only
> > >> - GET /api/ds
> > >> - GET /api/ds/:id
> > >> - every other GET api that TO supports (but make sure only GETs that
> are
> > >> reads. we have some GETs that do more than reading :) )
> > >>
> > >> ^^ maybe these 2 roles are "seeded" in the roles table. so out of the
> > box,
> > >> you get 2 roles and that 2nd role is very important because
> > >> ldap-authenticated users (with no user in the users table) get that
> > role...
> > >>
> > >> 2nd one - tenancy:
> > >>
> > >> how do we assign tenancy to these ldap-authenticated-has-no-user-row
> > >> users?
> > >> i would probably suggest they get assigned "root tenant" which is the
> > >> parent of all tenants thus eliminating scoping altogether.
> > >>
> > >> to summarize, i'd suggest ldap-authenticated user

Re: Access Control Model

[Jeremy Mitchell]

FYI, here is what our wholesale group has asked for regarding access
control.

https://docs.google.com/document/d/1kPw45vcqJfR3b_xmXMUxy9ibhWvuBJhZ7xWoi3JlC6E/edit#heading=h.i2v3ngs94gst

So with:

- a tenant hierarchy to restrict scope
- dynamic roles (not sure if this is something everyone thinks is a good
idea or not)

the requests of that google doc would be satisfied.

Jeremy


On Wed, Feb 22, 2017 at 10:33 AM, Jeremy Mitchell <mitchell...@gmail.com>
wrote:

> +1 on Dave's question. ldap is an edge case that we probably have to
> account for. so with this new approach, 2 things are checked - your role
> (what you can do) and your tenancy (scope of what you can do).
>
> first one - role:
>
> we'll need to think how ldap authenticated users (that don't have an
> actual row in the users table) are automatically assigned a role. Maybe the
> roles table is seeded with 2 roles like this: (remember, a role is a
> grouping of "capabilities" (or apis) )
>
> admin
> - GET /api/ds
> - GET /api/ds/:id
> - PUT /api/ds/:id
> - POST /api/ds
> - DELETE  /api/ds/:id
> - everything other api that TO supports
>
> read-only
> - GET /api/ds
> - GET /api/ds/:id
> - every other GET api that TO supports (but make sure only GETs that are
> reads. we have some GETs that do more than reading :) )
>
> ^^ maybe these 2 roles are "seeded" in the roles table. so out of the box,
> you get 2 roles and that 2nd role is very important because
> ldap-authenticated users (with no user in the users table) get that role...
>
> 2nd one - tenancy:
>
> how do we assign tenancy to these ldap-authenticated-has-no-user-row
> users? i would probably suggest they get assigned "root tenant" which is
> the parent of all tenants thus eliminating scoping altogether.
>
> to summarize, i'd suggest ldap-authenticated users (with no user entry in
> the user table) get:
>
> - the seeded read-only role
> - the root tenant
>
> And one more thought regarding roles. I mentioned 2 seeded roles (admin
> and R/O) but IMO an admin should be able to create N number of roles / api
> combinations. For example, maybe I have a someone that wants access to one
> and only one API, i could create the "john_role" that contains that one api
> and assign that role to john. My point is, the roles table could be
> dynamicwhoever administers TO can set up the roles as they see fit..
>
> Jeremy
>
> On Wed, Feb 22, 2017 at 9:11 AM, Eric Friedrich (efriedri) <
> efrie...@cisco.com> wrote:
>
>> Will the introduction of the new Access Control Service (and API) require
>> two new HTTP API calls for every call from the user to the API?
>>
>> I’d also like to second two of Hank’s requests below for #3 and #4.
>>
>> —Eric
>>
>> > On Feb 22, 2017, at 9:03 AM, Hank Beatty <hbea...@apache.org> wrote:
>> >
>> > Hi Naama,
>> >
>> > I like the idea of making Access Control hierarchical.
>> >
>> > I came up with some questions I thought we might think about:
>> >
>> > 1. Do the tenants become the equivalent of the "groups" we are using
>> today?
>> >
>> > 2. In your example, can there be children of 'Company C'?
>> >
>> > 3. Some of the APIs currently allow for 'token' authentication but, the
>> UI portion was never implemented (AFAIK). I would like to see the 'token'
>> authentication added so that scripts don't have to be run with user/pass.
>> >
>> > 4. I would like to see more than a single "root" user (perhaps a group
>> that one could assign users).
>> >
>> > Thanks,
>> > Hank
>> >
>> > On 02/21/2017 03:32 PM, Naama Shoresh wrote:
>> >> Hi,
>> >>
>> >> We've been considering the subject of authorization within TO, and have
>> >> phrased a concept we'd like to share and get some feedback about.
>> >> You can find it in the wiki
>> >> <https://cwiki.apache.org/confluence/pages/viewpage.action?
>> pageId=68715910>,
>> >> and also written below, for ease of use.
>> >> Your comments and insights are most welcome.
>> >>
>> >> =
>> >>
>> >> The access control model concept is constructed of two dimensions:
>> >> capabilities & data.
>> >> 1. CapabilitiesCheck if a user is allowed to perform an operation
>> >>
>> >> APIs are grouped to roles, and each user is assigned a set of roles
>> which
>> >> implies his allowed APIs.
>> >

Re: Access Control Model

[Jeremy Mitchell]

+1 on Dave's question. ldap is an edge case that we probably have to
account for. so with this new approach, 2 things are checked - your role
(what you can do) and your tenancy (scope of what you can do).

first one - role:

we'll need to think how ldap authenticated users (that don't have an actual
row in the users table) are automatically assigned a role. Maybe the roles
table is seeded with 2 roles like this: (remember, a role is a grouping of
"capabilities" (or apis) )

admin
- GET /api/ds
- GET /api/ds/:id
- PUT /api/ds/:id
- POST /api/ds
- DELETE  /api/ds/:id
- everything other api that TO supports

read-only
- GET /api/ds
- GET /api/ds/:id
- every other GET api that TO supports (but make sure only GETs that are
reads. we have some GETs that do more than reading :) )

^^ maybe these 2 roles are "seeded" in the roles table. so out of the box,
you get 2 roles and that 2nd role is very important because
ldap-authenticated users (with no user in the users table) get that role...

2nd one - tenancy:

how do we assign tenancy to these ldap-authenticated-has-no-user-row users?
i would probably suggest they get assigned "root tenant" which is the
parent of all tenants thus eliminating scoping altogether.

to summarize, i'd suggest ldap-authenticated users (with no user entry in
the user table) get:

- the seeded read-only role
- the root tenant

And one more thought regarding roles. I mentioned 2 seeded roles (admin and
R/O) but IMO an admin should be able to create N number of roles / api
combinations. For example, maybe I have a someone that wants access to one
and only one API, i could create the "john_role" that contains that one api
and assign that role to john. My point is, the roles table could be
dynamicwhoever administers TO can set up the roles as they see fit..

Jeremy

On Wed, Feb 22, 2017 at 9:11 AM, Eric Friedrich (efriedri) <
efrie...@cisco.com> wrote:

> Will the introduction of the new Access Control Service (and API) require
> two new HTTP API calls for every call from the user to the API?
>
> I’d also like to second two of Hank’s requests below for #3 and #4.
>
> —Eric
>
> > On Feb 22, 2017, at 9:03 AM, Hank Beatty  wrote:
> >
> > Hi Naama,
> >
> > I like the idea of making Access Control hierarchical.
> >
> > I came up with some questions I thought we might think about:
> >
> > 1. Do the tenants become the equivalent of the "groups" we are using
> today?
> >
> > 2. In your example, can there be children of 'Company C'?
> >
> > 3. Some of the APIs currently allow for 'token' authentication but, the
> UI portion was never implemented (AFAIK). I would like to see the 'token'
> authentication added so that scripts don't have to be run with user/pass.
> >
> > 4. I would like to see more than a single "root" user (perhaps a group
> that one could assign users).
> >
> > Thanks,
> > Hank
> >
> > On 02/21/2017 03:32 PM, Naama Shoresh wrote:
> >> Hi,
> >>
> >> We've been considering the subject of authorization within TO, and have
> >> phrased a concept we'd like to share and get some feedback about.
> >> You can find it in the wiki
> >>  action?pageId=68715910>,
> >> and also written below, for ease of use.
> >> Your comments and insights are most welcome.
> >>
> >> =
> >>
> >> The access control model concept is constructed of two dimensions:
> >> capabilities & data.
> >> 1. CapabilitiesCheck if a user is allowed to perform an operation
> >>
> >> APIs are grouped to roles, and each user is assigned a set of roles
> which
> >> implies his allowed APIs.
> >>
> >> Example:
> >>
> >> API  | Role
> >> -+-
> >> GET  /ds/:id | ds-read
> >> POST /ds/create  | ds-write
> >> POST /ds/:id/update  | ds-write
> >> POST /profile/:id/update | profile-write
> >>
> >> The  access is checked at the entry point. A user Joe which has the
> roles
> >> ds-read & ds-write is allowed to operate the following APIs:  /ds/:id,
> >> /ds/create and /ds/:id/update.
> >> 2. DataCheck if a user is allowed to access a specific set of data.
> >>
> >> Here is where the concept of *tenants* is introduced:
> >>
> >> Every "resource" in the  database is assigned  (delivery-services,
> servers,
> >> etc..) to  a tenant. A tenant is an organization in TC. It can be
> either a
> >> content-provider or an ISP. Tenants are hierarchical, where  the parent
> is
> >> conceptually assigned a super-set of all the resources of  its children.
> >>
> >> Each user belongs to one or more tenants. Only the tenant's resources
> are
> >> available for the tenant's users.
> >>
> >> * Note: for simplicitly, the example below refers to a single tenant per
> >> user.
> >>
> >> Example:
> >>
> >> Tenant table:
> >>
> >> ID  | Tenant-name | Parent-ID
> >> +-+---
> >> 1   | company A   | -
> >> 

Re: Upcoming TC 2.0 Release Branch

[Jeremy Mitchell]

Yeah, we've done a lot of Postgres testing against the master branch. I'm
afraid if Jan drops his PR in it will nullify a lot of that testing (as his
changes are pretty extensive). So my preference would be to create a 2.0
branch very soon (in a couple of days like Neuman said?) which should give
everyone time to merge any smaller / important PRs.

Jeremy

On Tue, Feb 21, 2017 at 7:52 AM, Jan van Doorn  wrote:

> I thought we didn’t want to muddy 2.0 with big new features and / or
> changes. That’s why I’m waiting.
>
> Rgds,
> JvD
>
> > On Feb 21, 2017, at 7:42 AM, Eric Friedrich (efriedri) <
> efrie...@cisco.com> wrote:
> >
> > It was mainly about JvD’s comment that he wants to get changes into 2.1.
> It seemed like he was waiting.
> >
> > —Eric
> >
> >> On Feb 21, 2017, at 8:42 AM, Dave Neuman  wrote:
> >>
> >> I think you are asking "Why not get everything into 2.0?"
> >> There are currently 20 open PRs and I know I don't have time to get
> through
> >> them all. That being said, there are some out there that I would like to
> >> see make it to 2.0, so I will try to get them tested and merged in the
> next
> >> couple days.
> >>
> >> Thanks,
> >> Dave
> >>
> >> On Mon, Feb 20, 2017 at 7:13 PM, Eric Friedrich (efriedri) <
> >> efrie...@cisco.com> wrote:
> >>
> >>> Any particular reason these changes don’t just go into 2.0?
> >>>
> >>> I think that smaller, more frequent releases pose less risk to deploy.
> >>> Aside from that are there other reasons to keep code “in your pocket”
> until
> >>> the release branch is cut?
> >>>
> >>> —Eric
> >>>
>  On Feb 20, 2017, at 6:08 PM, Dave Neuman  wrote:
> 
>  I'll take a look at open PRs tomorrow and see if there is anything I
> want
>  to make sure gets in. Other than that I am +1
> 
>  On Mon, Feb 20, 2017 at 16:06 Mark Torluemke 
> >>> wrote:
> 
> > Agree -- I think sooner-is-better for branching 2.0.
> >
> > Cheers,
> > Mark
> >
> > On Mon, Feb 20, 2017 at 3:57 PM, Jan van Doorn 
> wrote:
> >
> >> +100 !
> >>
> >> I really want to get some changes in to 2.1.
> >>
> >> Thanks,
> >> JvD
> >>
> >>> On Feb 20, 2017, at 1:42 PM, Eric Friedrich 
> >>> wrote:
> >>>
> >>> Hey All-
> >>> Its about time to cut our first branch in the 2.0 series and
> starting
> >>> testing release candidates.
> >>>
> >>> TC1.8 is not quite yet through the incubator voting process, but it
> >> appears
> >>> that approval is hopefully quite close. No changes have gone into
> 1.8
> > in
> >>> the past few months, so on top of the move to Postgres we have
> quite
> > the
> >>> set of changes in this upcoming release.
> >>>
> >>>
> >>> Unless I hear strong opinions otherwise in the next 2-3 days, I'll
> cut
> >> the
> >>> TC2.0 release branch later this week and bump the master version
> > numbers
> >> up
> >>> to 2.1
> >>>
> >>>
> >>> Thanks,
> >>> Eric
> >>> Release Manager-elect
> >>
> >>
> >
> >>>
> >>>
> >
>
>

Re: Github PR suggestion

[Jeremy Mitchell]

It kinda seems like our contributing.md file already implies that an issue
is required -
https://github.com/apache/incubator-trafficcontrol/blob/master/CONTRIBUTING.md

Obviously, it slows down the process to have to create a jira issue,
discuss on the mailing listfor EVERY PR.

Maybe issue / mailing list discussion required for new features and
optional for bugs?

On Wed, Feb 15, 2017 at 8:07 PM, David Neuman <david.neuma...@gmail.com>
wrote:

> I am -1 on it being "required" however,  I am ok with saying we prefer it.
>
>
> On Wed, Feb 15, 2017 at 20:04 Jeremy Mitchell <mitchell...@gmail.com>
> wrote:
>
> > Should we require a Jira issue for each PR? I don't think it's a bad idea
> > personally. And if we do require a Jira issue, it would be nice imo to
> see
> > each PR entitled like "[TC-XXX] - fixes blah blah"
> >
> > Jeremy
> >
> > On Tue, Feb 14, 2017 at 2:16 PM, Leif Hedstrom <zw...@apache.org> wrote:
> >
> > > Hi all,
> > >
> > > since we now have a process which lets us create Github PRs, without
> > first
> > > filing a bug report / issue, I’ve come to find that it’s many times
> > > difficult to know what “issue” a particular PR is addressing. So, I
> would
> > > like to suggest that we all make an effort to put more Description text
> > in
> > > Github PRs, which do not have a Github Issue filed already.
> > >
> > > I’m also writing up a little bit of this into the CONTRIBUTING.md.
> > >
> > > Thanks,
> > >
> > > — Leif
> > >
> > >
> >
>

Re: Possible bug in traffic ops when sending an e-mail

[Jeremy Mitchell]

I'll have to defer to someone else as most of that was way over my head but
I do know we have this ticket outstanding:
https://issues.apache.org/jira/browse/TC-74

and also, this PR was merged a while back regarding $SIG{CHLD} -
https://github.com/apache/incubator-trafficcontrol/pull/155

I'm not really sure if it's a good or bad idea to wrap every call to
$self->mail(...);
as you suggested. maybe others have thoughts on that.

preferably, it would be nice to get TC-74 done and remove our dependency on
an outdated library.

jeremy


On Tue, Feb 7, 2017 at 4:27 AM, Oren Shemesh  wrote:

> Hi,
>
> I'm looking for someone to form an opinion about a problem I believe I have
> found, and a fix.
>
> The problem manifests itself as an error when TO attempts to send an
> E-mail.
>
> The details:
> When traffic ops runs, it daemonizes itself, and as part of that, it does
> $SIG{CHLD} = 'IGNORE'; :
>
> https://github.com/apache/incubator-trafficcontrol/blob/
> master/traffic_ops/app/lib/MojoPlugins/Daemonize.pm#L28
>
> But, when sending a mail, TO uses Mojolicous::Plugin::Mail, which in turn
> is a wrapper for the standard (And Obsolete) perl module MIME::Lite.
>
> MIME::Lite sends a mail by running the following piece of code (Snippet
> from app/local/lib/perl5/MIME/Lite.pm):
>
>my $pid = open SENDMAIL, "|-";
> defined($pid) or die "open of pipe failed: $!\n";
> if ( !$pid ) {### child
> exec(@cmd) or die "can't exec $p{Sendmail}: $!\n";
> ### NOTREACHED
> } else {  ### parent
> $self->print( \*SENDMAIL );
> close SENDMAIL || die "error closing $p{Sendmail}: $! (exit
> $?)\n";
> $return = 1;
> }
>
> The problem with this code is that assumes that the SIGCHLD handler is left
> at it's DEFAULT handling, which means that when a child process terminates,
> the parent process can reap it during the 'close' statement.
> Traffic Ops runs with the SIGCHLD handler set to IGNORE, which means that
> when a child process dies, the parent process forgets about it immediately.
> So, if sendmail runs fast enough and finishes before the 'close' command is
> executed, the close will fail to find the child process, and the error
> propagates all the way to the calling code, looking something like this:
>
> Traffic Ops fatal error occurred while processing your request.
> Error at line 2732 ( close SENDMAIL || die "error closing $p{Sendmail}: $!
> (exit $?)\n";)
> error closing /usr/lib/sendmail: No child processes (exit -1)
>
> It should be noted that we are running TO on single-core machines for
> development, which means that the code is much more likely to fail in this
> manner (In machines with more cores, the parent process usually does not
> get suspended when sendmail is spawned, hence it reaches the 'close'
> command immediately and waits for sendmail to finish properly).
>
> I am not sure why TO needs to completely ignore SIGCHLD, and I am not sure
> if it is reasonable to ask from a module such as MIME::Lite to work when
> SIGCHLD is set to IGNORE.
> However, MIME::Lite is not being maintained any more (The owner recommends
> using better modules, see this
> ),
> and
> the latest Mojolicous::Plugin::Mail version still uses MIME::Lite.
> (I guess Mojolicous is not very actively maintained these days).
> Which means that until we switch to the experimental, Angular-based,
> Traffic Ops, we are stuck with the current MIME::Lite code.
>
> Maybe a good fix for this issue is to not set SIGCHLD to IGNORE while
> daemonizing. However, this means that every time we create a new process,
> we must waitpid() for it, otherwise we will accumulate zombies.
> I guess it is not very practical to ensure this across the entire code base
> of TO (And, as we have seen, all Perl modules that it uses).
>
> So the low-footprint fix I want to make is to wrap every call to
> $self->mail(...) done in our Email.pm
>  master/traffic_ops/app/lib/MojoPlugins/Email.pm>
> code, like this:
>
> $SIG{CHLD} = 'DEFAULT';
> $self->mail(...);
> $SIG{CHLD} = 'IGNORE';
>
> I tested this, and it fixes the problem.
>
> Thoughts ?
>
> I plan to create a JIRA and a PR, but I wanted to get someone's more expert
> view on this before I go on.
> Being a bit of a novice, maybe I am missing some key thing here.
>
> Thanks ,Oren.
> --
>
> *Oren Shemesh*
> Qwilt | Work: +972-72-2221637| Mobile: +972-50-2281168 | or...@qwilt.com
> 
>

Re: TO - Queue server updates on delivery service servers only?

[Jeremy Mitchell]

The more I think about this, the more I think it's not as straightforward
as I think to queue updates for a delivery service (meaning to queue
updates for all the caches employed by the ds).

For example, imagine you had a delivery service that employed 10
cachesand you removed 2 caches from the ds...then queue'd updates for
the ds.this would queue updates for the 8 remaining caches but really
the 2 caches you removed are the ones that need the updates...

So unless told otherwise or someone has a solution to this problem (I can't
think of one currently), I'm gonna hold off on that issue.

Jeremy

On Wed, Feb 1, 2017 at 8:58 PM, Jeremy Mitchell <mitchell...@gmail.com>
wrote:

> Created this issue. Would like input from more people to ensure this is a
> good idea and I'm not overlooking something...
>
> https://issues.apache.org/jira/browse/TC-129
>
> On Tue, Jan 31, 2017 at 10:47 AM, Eric Friedrich (efriedri) <
> efrie...@cisco.com> wrote:
>
>> Yes, when modifying a DS, it would be useful to have an API to queue on
>> just servers relevant to that DS.
>>
>> —Eric
>>
>> > On Jan 31, 2017, at 12:36 PM, Jeremy Mitchell <mitchell...@apache.org>
>> wrote:
>> >
>> > Currently, you can queue updates:
>> >
>> > - for a specific server
>> > - for a specific cachegroup (all the servers in that cachegroup)
>> > - for a specific cdn (all the servers in that cdn)
>> >
>> > but you can't queue updates for:
>> >
>> > - a specific delivery service (all the servers explicitly (edges) or
>> > implicitly (mids) assigned to that DS)
>> >
>> > Does it make sense to add this functionality? At least to the API?
>> >
>> > I "think" when a delivery service change is made, it is common practice
>> to
>> > queue updates on the ENTIRE cdn that the DS belongs to. This seems like
>> it
>> > would unnecessarily queue updates for a lot of servers that don't
>> belong to
>> > the DS.
>> >
>> > I know we may move away from the queue update approach at some point but
>> > does this functionality make sense in the short term?
>> >
>> > Jeremy
>>
>>
>

Re: TO - Queue server updates on delivery service servers only?

[Jeremy Mitchell]

Created this issue. Would like input from more people to ensure this is a
good idea and I'm not overlooking something...

https://issues.apache.org/jira/browse/TC-129

On Tue, Jan 31, 2017 at 10:47 AM, Eric Friedrich (efriedri) <
efrie...@cisco.com> wrote:

> Yes, when modifying a DS, it would be useful to have an API to queue on
> just servers relevant to that DS.
>
> —Eric
>
> > On Jan 31, 2017, at 12:36 PM, Jeremy Mitchell <mitchell...@apache.org>
> wrote:
> >
> > Currently, you can queue updates:
> >
> > - for a specific server
> > - for a specific cachegroup (all the servers in that cachegroup)
> > - for a specific cdn (all the servers in that cdn)
> >
> > but you can't queue updates for:
> >
> > - a specific delivery service (all the servers explicitly (edges) or
> > implicitly (mids) assigned to that DS)
> >
> > Does it make sense to add this functionality? At least to the API?
> >
> > I "think" when a delivery service change is made, it is common practice
> to
> > queue updates on the ENTIRE cdn that the DS belongs to. This seems like
> it
> > would unnecessarily queue updates for a lot of servers that don't belong
> to
> > the DS.
> >
> > I know we may move away from the queue update approach at some point but
> > does this functionality make sense in the short term?
> >
> > Jeremy
>
>

Re: Invalidate Content Error

[Jeremy Mitchell]

I created this issue to account for seed data lost in the transition from
1.8 to 2.0

https://issues.apache.org/jira/browse/TC-126

On Wed, Feb 1, 2017 at 10:26 AM, Jeremy Mitchell <mitchell...@gmail.com>
wrote:

> hmmm. i think  you've stumbled on a larger issue. the move from 1.8 to 2.0
> (master) included the consolidation of migration files into
> create_tables.sql file but.some of those migrations represented the
> seeding of data (i.e. 2015120800_add_job_status.sql) which would not
> be represented in create_tables.sql.
>
> i'll look into that more. thanks.
>
> jeremy
>
> On Wed, Feb 1, 2017 at 9:05 AM, Oren Shemesh <or...@qwilt.com> wrote:
>
>> Indeed I failed to notice that the
>> file traffic_ops/app/db/migrations/2015120800_add_job_status.sql does
>> not exist anymore in master,
>> and the seeds.sql file in master does not contain any insert to a
>> job-related table to compensate for that.
>> So the job_agent and job_status tables are empty after a fresh install.
>> Which means that Invalidate Content probably does not work out of the box
>> on master.
>> (I did not test master myself. Currently, as a novice, I am still learning
>> the product, and so I stick to the last released version, 1.7)
>> Which means that Jeremy's suggested fix is very much needed.
>>
>> I think nobody suggests that TC Content Invalidation should depend on the
>> user having to manually manipulate the DB, even if that user is novice and
>> just did a fresh TC install ;-)
>>
>> On Wed, Feb 1, 2017 at 5:47 PM, Jeremy Mitchell <mitchell...@gmail.com>
>> wrote:
>>
>> > It works IF he does a manual insert into the job_agent table and it so
>> > happens in master that insert is assigned an id of 1. Is that correct,
>> > Oren? If so, I don't "think" imo you should have to do any manual
>> inserts
>> > but i suppose that can be debated. I'll leave it alone and if anyone
>> thinks
>> > the job_agent / job_status table should be "seeded" on install, they can
>> > create an issue / PR.
>> >
>> > Jeremy
>> >
>> > On Wed, Feb 1, 2017 at 8:31 AM, Dave Neuman <neu...@apache.org> wrote:
>> >
>> > > I am pretty sure he already said it works in master?
>> > >
>> > > On Wed, Feb 1, 2017 at 8:30 AM, Jeremy Mitchell <
>> mitchell...@gmail.com>
>> > > wrote:
>> > >
>> > > > So going forward with the master branch (it's too late for 1.8 so
>> your
>> > > > workaround will have to suffice), I think 2 lines need to be added
>> to
>> > > > seeds.sql so invalidate content works on a fresh install:
>> > > >
>> > > > - an insert into the job_agent table (so a job agent with id=1
>> exists)
>> > > > - an insert into the the job_status table with name=PENDING
>> > > >
>> > > > I'll create an issue for that.
>> > > >
>> > > > Jeremy
>> > > >
>> > > > On Wed, Feb 1, 2017 at 8:17 AM, Dave Neuman <neu...@apache.org>
>> wrote:
>> > > >
>> > > > > As for the release notes, since Dan is our release manager, I
>> would
>> > get
>> > > > > them over to him.
>> > > > >
>> > > > > On Wed, Feb 1, 2017 at 8:11 AM, Oren Shemesh <or...@qwilt.com>
>> > wrote:
>> > > > >
>> > > > > > Thanks Jeremy :-)
>> > > > > >
>> > > > > > On Wed, Feb 1, 2017 at 5:09 PM, Jeremy Mitchell <
>> > > mitchell...@gmail.com
>> > > > >
>> > > > > > wrote:
>> > > > > >
>> > > > > > > I don't think jobs are ever programmatically removed from the
>> > jobs
>> > > > > table
>> > > > > > > nor are their statuses changed. They always remain in a
>> pending
>> > > > state.
>> > > > > > >
>> > > > > > > SELECT COUNT(*) FROM job WHERE status NOT IN ( SELECT id FROM
>> > > > > job_status
>> > > > > > > WHERE name = 'PENDING' )
>> > > > > > > 0
>> > > > > > >
>> > > > > > > Like I mentioned earlier, I think there was a lot more planned
>> > for
>> > > > jobs
>> > > > > > but
>> > > > > > > as it stands today, the job

Re: Invalidate Content Error

[Jeremy Mitchell]

hmmm. i think  you've stumbled on a larger issue. the move from 1.8 to 2.0
(master) included the consolidation of migration files into
create_tables.sql file but.some of those migrations represented the
seeding of data (i.e. 2015120800_add_job_status.sql) which would not be
represented in create_tables.sql.

i'll look into that more. thanks.

jeremy

On Wed, Feb 1, 2017 at 9:05 AM, Oren Shemesh <or...@qwilt.com> wrote:

> Indeed I failed to notice that the
> file traffic_ops/app/db/migrations/2015120800_add_job_status.sql does
> not exist anymore in master,
> and the seeds.sql file in master does not contain any insert to a
> job-related table to compensate for that.
> So the job_agent and job_status tables are empty after a fresh install.
> Which means that Invalidate Content probably does not work out of the box
> on master.
> (I did not test master myself. Currently, as a novice, I am still learning
> the product, and so I stick to the last released version, 1.7)
> Which means that Jeremy's suggested fix is very much needed.
>
> I think nobody suggests that TC Content Invalidation should depend on the
> user having to manually manipulate the DB, even if that user is novice and
> just did a fresh TC install ;-)
>
> On Wed, Feb 1, 2017 at 5:47 PM, Jeremy Mitchell <mitchell...@gmail.com>
> wrote:
>
> > It works IF he does a manual insert into the job_agent table and it so
> > happens in master that insert is assigned an id of 1. Is that correct,
> > Oren? If so, I don't "think" imo you should have to do any manual inserts
> > but i suppose that can be debated. I'll leave it alone and if anyone
> thinks
> > the job_agent / job_status table should be "seeded" on install, they can
> > create an issue / PR.
> >
> > Jeremy
> >
> > On Wed, Feb 1, 2017 at 8:31 AM, Dave Neuman <neu...@apache.org> wrote:
> >
> > > I am pretty sure he already said it works in master?
> > >
> > > On Wed, Feb 1, 2017 at 8:30 AM, Jeremy Mitchell <mitchell...@gmail.com
> >
> > > wrote:
> > >
> > > > So going forward with the master branch (it's too late for 1.8 so
> your
> > > > workaround will have to suffice), I think 2 lines need to be added to
> > > > seeds.sql so invalidate content works on a fresh install:
> > > >
> > > > - an insert into the job_agent table (so a job agent with id=1
> exists)
> > > > - an insert into the the job_status table with name=PENDING
> > > >
> > > > I'll create an issue for that.
> > > >
> > > > Jeremy
> > > >
> > > > On Wed, Feb 1, 2017 at 8:17 AM, Dave Neuman <neu...@apache.org>
> wrote:
> > > >
> > > > > As for the release notes, since Dan is our release manager, I would
> > get
> > > > > them over to him.
> > > > >
> > > > > On Wed, Feb 1, 2017 at 8:11 AM, Oren Shemesh <or...@qwilt.com>
> > wrote:
> > > > >
> > > > > > Thanks Jeremy :-)
> > > > > >
> > > > > > On Wed, Feb 1, 2017 at 5:09 PM, Jeremy Mitchell <
> > > mitchell...@gmail.com
> > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > I don't think jobs are ever programmatically removed from the
> > jobs
> > > > > table
> > > > > > > nor are their statuses changed. They always remain in a pending
> > > > state.
> > > > > > >
> > > > > > > SELECT COUNT(*) FROM job WHERE status NOT IN ( SELECT id FROM
> > > > > job_status
> > > > > > > WHERE name = 'PENDING' )
> > > > > > > 0
> > > > > > >
> > > > > > > Like I mentioned earlier, I think there was a lot more planned
> > for
> > > > jobs
> > > > > > but
> > > > > > > as it stands today, the jobs table is currently for "invalidate
> > > > > content"
> > > > > > > (purge) jobs ONLY and the jobs go in to that table and never
> come
> > > > out.
> > > > > > It's
> > > > > > > kind of like the roach motel :)
> > > > > > > https://www.youtube.com/watch?v=jKhGHxO-woc=youtu.
> > be=27
> > > > > > >
> > > > > > > Anyhow, it's important to remember that when initiating an
> > > > "invalidate
> > > > > > > content" re

Re: Invalidate Content Error

[Jeremy Mitchell]

In my opinion, the job agent id should have been passed to the newjob()
subroutine rather than depending on a hard-coded value...actually, it looks
like that was the intent but the $agent variable never got used.

https://github.com/apache/incubator-trafficcontrol/blob/master/traffic_ops/app/lib/UI/Job.pm#L42

Also, I don't think the concept of jobs, job agents, etc was ever fully
fleshed out (somebody correct me if i'm wrong), therefore, it would be my
opinion that we eventually kill these tables:

- job_agent
- job_status
- job_result

this would eliminate the job_agent FK from the job table and then this
problem of potentially using an job agent ID that doesn't exist goes bye
bye...

also, fyi, i believe the job table really only contains "purge" aka
"invalidate content" jobs...(defined by keyword=purge).

Jeremy

On Tue, Jan 31, 2017 at 12:42 PM, Dave Neuman  wrote:

> Thanks for the investigation.  Sounds like there is definitely a bug
> there.  I would vote don't worry about fixing the issue in 1.8 since it is
> fixed in master.  We should, however, make a release note or something
> explaining the issue and the work around.
>
> --Dave
>
> On Tue, Jan 31, 2017 at 12:32 PM, Oren Shemesh  wrote:
>
> > Continuing the investigation of this issue, I have found what I believe
> to
> > be an inconsistency in the code that handles content invalidation.
> > To the best of my understanding, the bug exists in 1.7.x, and in the
> latest
> > 1.8.x as well, and was only fixed in master, as part of the move to
> > postgresql.
> >
> > The bug prevents you from adding a job, you get an immediate error in the
> > UI.
> >
> > Question: Can anyone attest to Content Invalidation working in a fresh
> > install of TC 1.7 or 1.8 ?
> >
> > Another question: Given that the problem probably does not exist in
> > 'master', do we care about fixing it in the 1.8.x train (Or a following
> > train, non-postgresql) ?
> > If so, I will issue a JIRA.
> >
> > The details:
> >
> > I believe there is an inconsistency between the code that creates the TO
> DB
> > during a clean install, and the code that depends on it.
> >
> > Creating the TO DB is done by traffic_ops/app/db/create_tables.sql. In
> the
> > mysql version (1.7 and 1.8), it creates the 'job_agent' table and the
> > 'job_status' table with table options AUTO_INCREMENT=2
> > and AUTO_INCREMENT=5, respectively:
> >
> > *CREATE TABLE `job_agent`* (
> >   `id` int(11) NOT NULL AUTO_INCREMENT,
> > ...
> > ) ENGINE=InnoDB *AUTO_INCREMENT=2* DEFAULT CHARSET=latin1;
> >
> > *CREATE TABLE `job_status`* (
> >   `id` int(11) NOT NULL AUTO_INCREMENT,
> > ...
> > ) ENGINE=InnoDB *AUTO_INCREMENT=5* DEFAULT CHARSET=latin1;
> >
> > This means that when the script that populates these tables
> > (traffic_ops/app/db/migrations/2015120800_add_job_status.sql) is
> run,
> > it creates entries with an id that begins with 2 and 5, respectively.
> >
> > However, the code that attempts to add an entry to the 'job' table,
> assumes
> > that the id numbers in both tables, begins at 1.
> >
> > From traffic_ops/app/lib/UI/Job.pm:
> >
> > sub newjob {
> > ...
> > my $*status = 1*;
> > ...
> > my $insert = $self->db->resultset('Job')->create(
> > {
> > *agent   => 1,*
> > object_type => $object_type,
> > object_name => $object_name,
> > entered_time=> $entered_time,
> > keyword => $keyword,
> > parameters  => $parameters,
> > asset_url   => $org_server_fqdn,
> > asset_type  => $asset_type,
> > *status  => $status*,
> > job_user=> $user,
> > start_time  => $start_time_gmt,
> > job_deliveryservice => $ds->id,
> > }
> >
> > As you can see, both the 'agent' and the 'status' fields are set to a
> > hard-coded value of 1, which cannot exist in tables that are set to have
> an
> > auto-incremented value of 'id' that begins with 2 or 5.
> >
> > When looking at the traffic_ops/app/db/create_tables.sql script in
> > 'master', which is now adapted to postgresql, it seems that it is totally
> > different code, but the inconsistency is now gone, because the numbering
> > starts at 1:
> >
> > CREATE TABLE job_agent (
> > id bigint NOT NULL,
> > name text,
> > description text,
> > active integer DEFAULT 0 NOT NULL,
> > last_updated timestamp with time zone DEFAULT now()
> > );
> >
> > CREATE SEQUENCE job_agent_id_seq
> > *START WITH 1*
> > INCREMENT BY 1
> > NO MINVALUE
> > NO MAXVALUE
> > CACHE 1;
> >
> > ALTER SEQUENCE job_agent_id_seq OWNED BY job_agent.id;
> >
> > Regards, Oren.
> >
> > On Thu, Jan 26, 2017 at 5:36 PM, Dave Neuman  wrote:
> >
> > > Good to hear.  It's too bad that we have some routes requiring a
> specific
> > > ID from the database, when you come across those can you open a Jira
> > issue
> > > so we know they need to be fixed?
> > > Thanks,
> > > Dave
> > >
> > > On Thu, Jan 26, 2017 at 5:17 AM, 

Re: [VOTE] Release Apache Traffic Control 1.8.0-incubating (RC9)

[Jeremy Mitchell]

+1

tested TO UI, TO API and TP (traffic portal).

On Fri, Jan 27, 2017 at 9:46 AM, Dan Kirkwood  wrote:

> +1
>
> -- was assuming as release manager I didn't get a vote,  but not true?
>
> I tested:
> -  signature and checksums good
> -  build from gitrepo and RC9 tag
> - scanning source with `rat` tool shows no license issues
> -  install traffic_ops on new Centos 7.2 VM
>  -  setup new mysql,  loaded with test data
>  -  login from UI,  navigating parts of UI
>
>
> On Wed, Jan 25, 2017 at 9:15 AM, Dan Kirkwood  wrote:
> > Hello All,
> >
> > I've prepared another release for v1.8.0 (RC9)
> >
> > Changes since 1.7.0:
> > https://github.com/apache/incubator-trafficcontrol/
> compare/RELEASE-1.7.0...RELEASE-1.8.0-RC9
> >
> > This corresponds to git:
> >   Hash: 2301659f699948d9944cc07bc92b9f6a56bc4678
> >   Tag: RELEASE-1.8.0-RC9
> >
> > Which can be verified with the following:git tag -v RELEASE-1.8.0-RC9
> >
> > My code signing key is available here:
> > http://keys.gnupg.net/pks/lookup?search=0x4587A8F0=vindex
> >
> > Make sure you refresh from a key server to get all relevant signatures.
> >
> > The source .tar.gz file, pgp signature (.asc signed with my key from
> > above), and md5 and sha1 checksums are provided here:
> > https://dist.apache.org/repos/dist/dev/incubator/
> trafficcontrol/1.8.0/RC9
> >
> > More Apache License compliance changes are included in this RC.
> > Several third-party javascript files were removed that were deemed
> > unused.
> >
> > The vote will remain open until Friday,  Jan 27, 2017.
> >
> > Thanks!
>

Re: TC 2.0.x branch

[Jeremy Mitchell]

Dan has submitted a PR to merge the postgres branch into master -
https://github.com/apache/incubator-trafficcontrol/pull/168

In doing so, he had to work through a number of conflicts. I am going to
give his PR a once over and then hopefully merge it today.

Jeremy

On Fri, Jan 6, 2017 at 12:04 PM, Eric Friedrich (efriedri) <
efrie...@cisco.com> wrote:

> Following some more discussion on Slack, we are still planning to merge
> the postgres branch into master today.
>
> However, we will defer cutting the 2.0 release branch until after the 1.8
> release passes apache incubator voting.
>
> This will cut down on the amount of back porting needed, by keeping the
> number of active branches down.
>
> Any objections?
> —Eric
>
> > On Jan 6, 2017, at 8:27 AM, Jan van Doorn <j...@knutsel.com> wrote:
> >
> > We are moving away from MySQL, so from 2.0 on, we only support  Postgres.
> >
> > And, to emphasize, master will be moving to Postgres as well, all
> releases after 2.0 will be Postgres.
> >
> > There will be a migration included that migrates your existing database.
> >
> > Rgds,
> > JvD
> >
> >> On Jan 6, 2017, at 4:44 AM, Eric Friedrich (efriedri) <
> efrie...@cisco.com> wrote:
> >>
> >> Will 2.0 still support MySQL or just Postgres?
> >>
> >> Will the 2.0 release include auto-migration from MySQL to Postgres?
> >>
> >> Any other changes coming in that branch other than the move to a
> different DB?
> >>
> >> —Eric
> >>
> >>> On Jan 5, 2017, at 6:33 PM, Jeremy Mitchell <mitchell...@apache.org>
> wrote:
> >>>
> >>> As discussed in the traffic-control-cdn#dev slack channel, the plan is
> to
> >>> merge the contents of the postgres branch (
> >>> https://github.com/apache/incubator-trafficcontrol/tree/postgres) to
> the
> >>> master branch by Friday, 1/6/16 end of day and subsequently cut a 2.0.x
> >>> branch.
> >>>
> >>> Included in 2.0.x will be any commits applied to master after 1.8.x
> was cut
> >>> - https://github.com/apache/incubator-trafficcontrol/
> compare/1.8.x...master
> >>>
> >>> plus, of course, changes made to support Traffic Ops on Postgres.
> >>>
> >>> This also means if you have any outstanding PR's (
> >>> https://github.com/apache/incubator-trafficcontrol/pulls) that you
> want in
> >>> 2.0.x, reach out to a committer to get them merged. Otherwise, they
> will
> >>> have to target the 2.1 release.
> >>>
> >>> Questions? Concerns?
> >>>
> >>> thanks.
> >>
> >
>
>

Re: regex_revalidate.config

[Jeremy Mitchell]

derek gelinas is working on a PR that "scopes" these config files. i.e.
some config files are at the server level, profile level or cdn level.
regex_revalidate.config happens to be at the cdn level. Here is his PR that
I think will be targeted for 2.1.

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

what this means is that if a server needs to fetch regex_revalidate.config,
it would fetch it like this
to-domain.com/api/1.2/cdn/cdn1/configfiles/ats/regex_revalidate.config and
if this file is served from a cache of its own, the request by X number of
servers to get the same file would take advantage of caching to reduce the
delivery time. sooo.maybe it's a second the first time and a few ms
each subsequent time...

hope that made sense.

jeremy



On Tue, Jan 3, 2017 at 7:05 PM, Steve Malenfant <smalenf...@gmail.com>
wrote:

> Jeremy,
>
> I missed this email. Seems like close to a second is quite slow. Is the
> regex_revalidate.config per server? It seems like it was per CDN before.
>
> On Tue, Dec 13, 2016 at 1:02 PM, Jeremy Mitchell <mitchell...@gmail.com>
> wrote:
>
> > Obviously, it is going to be slower to pull regex_revalidate.config thru
> TO
> > as opposed to pulling it off the file system. The question is...is this
> > acceptable?
> >
> > curling to
> > https://to.domain.com/Trafficserver-Snapshots/cdn-
> > name/regex_revalidate.config
> >
> > time_namelookup: 0.005
> > time_connect: 0.058
> > time_appconnect: 0.000
> > time_pretransfer: 0.000
> > time_redirect: 0.000
> > time_starttransfer: 0.000
> > --
> > time_total: 0.063
> >
> > curling to
> > https://to.domain.com/genfiles/view/server-host-
> > name/regex_revalidate.config
> >
> > time_namelookup: 0.005
> > time_connect: 0.062
> > time_appconnect: 0.307
> > time_pretransfer: 0.307
> > time_redirect: 0.000
> > time_starttransfer: 0.781
> > --
> > time_total: 0.781
> >
> > On Mon, Dec 12, 2016 at 9:03 PM, Steve Malenfant <smalenf...@gmail.com>
> > wrote:
> >
> > > I'm OK with this. I think most of the revalidate functionality is not
> > well
> > > understood here... I guess I need to look into the API since we are
> using
> > > the UI to issue revalidate today.
> > >
> > > On Mon, Dec 12, 2016 at 5:43 PM, Jeremy Mitchell <
> mitchell...@apache.org
> > >
> > > wrote:
> > >
> > > > I've created an issue to solicit some feedback regarding the removal
> of
> > > > regex_revalidate.config from the file system. If you are not aware,
> > > > regex_revalidate.config is the ATS config file used to "invalidate
> > > > content". Storing regex_revalidate.config on the file system makes it
> > > > harder to set up a highly-available, redundant traffic ops.
> > > >
> > > > https://issues.apache.org/jira/browse/TC-68
> > > >
> > > > Thanks!
> > > >
> > >
> >
>

TC 2.0.x branch

[Jeremy Mitchell]

As discussed in the traffic-control-cdn#dev slack channel, the plan is to
merge the contents of the postgres branch (
https://github.com/apache/incubator-trafficcontrol/tree/postgres) to the
master branch by Friday, 1/6/16 end of day and subsequently cut a 2.0.x
branch.

Included in 2.0.x will be any commits applied to master after 1.8.x was cut
- https://github.com/apache/incubator-trafficcontrol/compare/1.8.x...master

plus, of course, changes made to support Traffic Ops on Postgres.

This also means if you have any outstanding PR's (
https://github.com/apache/incubator-trafficcontrol/pulls) that you want in
2.0.x, reach out to a committer to get them merged. Otherwise, they will
have to target the 2.1 release.

Questions? Concerns?

thanks.

Re: cdn table and domain_name parameter?

[Jeremy Mitchell]

I agree, it would be great to drop the profile column from the
deliveryservice table (and add domain_name to cdn table). In my mind, a
profile is really a "server profile" and intended for servers (caches). In
addition, by allowing users to select a profile for a deliveryservice, we
introduce the possibility of human-error (they select the wrong CCR
profile) which can cause issues for the CDN.

On Mon, Dec 26, 2016 at 9:43 AM, Mark Torluemke 
wrote:

>
> On Mon, Dec 26, 2016 at 9:20 AM, Jan van Doorn  wrote:
>
>> > or its own table entirely, with a link to the 'cdn' table.
>>
>> Do you think we should consider supporting multiple domains per CDN in
>> the future? Or is there another use case?
>>
>>
> That's the use case. I'd love to hear folks from the community weigh in,
> as it's been a topic for discussion many times, but we haven't had an
> explicit request for it.
>
>
>> Rgds,
>> JvD
>>
>> > On Dec 26, 2016, at 09:13, Mark Torluemke 
>> wrote:
>> >
>> > Agree, I also believe the CCR profile <> deliveryservice mapping is
>> superfluous, now that there is a link from cdn <> deliveryservice. This was
>> discussed when the 'cdn' table was being implemented, but perhaps too late
>> into the implementation phase. Further, I also agree that the domain_name
>> parameter should be moved to the 'cdn' table, or its own table entirely,
>> with a link to the 'cdn' table.
>> >
>> > On Mon, Dec 26, 2016 at 8:14 AM, Jan van Doorn > > wrote:
>> > Looking at the ATS 6.2 support for TO which requires a deliveryservice
>> to profile mapping, and was wondering why we still have the profile column
>> (CCR Profile) in deliveryservice?
>> >
>> > At first glance it seems to be used for the domain_name parameter only
>> (?), and that could (should?) be moved to the cdn table? Not sure if this
>> was considered when the cdn table was added and decided against for a good
>> reason?
>> >
>> > Cheers,
>> > JvD
>> >
>> >
>>
>>
>

regex_revalidate.config

[Jeremy Mitchell]

I've created an issue to solicit some feedback regarding the removal of
regex_revalidate.config from the file system. If you are not aware,
regex_revalidate.config is the ATS config file used to "invalidate
content". Storing regex_revalidate.config on the file system makes it
harder to set up a highly-available, redundant traffic ops.

https://issues.apache.org/jira/browse/TC-68

Thanks!

Re: Proposed change to Deliverservice API

[Jeremy Mitchell]

Let's look at an example of using ID's versus names for POSTS (creates)

Here is the region table. A region belongs to a division.

mysql> desc region;
+--+-+--+-+---+-+
| Field| Type| Null | Key | Default   | Extra
|
+--+-+--+-+---+-+
| id   | int(11) | NO   | PRI | NULL  |
auto_increment  |
| name | varchar(45) | NO   | UNI | NULL  |
|
| division | int(11) | NO   | MUL | NULL  |
|
| last_updated | timestamp   | YES  | | CURRENT_TIMESTAMP | on update
CURRENT_TIMESTAMP |
+--+-+--+-+---+-+
4 rows in set (0.01 sec)

Currently, if you want to create a region, you have to provide a division
"id" (as opposed to a division name)

POST /api/version/regions

{
name: "myregion",
division: 2
}

This allows the json to go directly into the table with one insert query.

if we use a division name instead, like this

{
name: "myregion",
division: "central"
}

then 2 queries have to happen on the server side. 1 query to fetch the
division id for "central" and then the insert query to create the region.

To do this right, imo, the ID for the central division WOULD be "central"
instead of the number 2 - and this is why natural keys make a lot of sense.
So rather than changing the api to accept cdn name, profile name, and type
name, continue to send thru the ids and we need to make the effort to get
to natural keys.

my 2 cents

On Wed, Nov 30, 2016 at 7:53 AM, Dave Neuman  wrote:

> Thanks Derek, it's actually already done, but then it dawned on me that it
> might break others, which is why I posted.
>
> On Wed, Nov 30, 2016 at 7:51 AM, Gelinas, Derek  >
> wrote:
>
> > I've already got a bit of code you can use for just that if you like.
> > Doing the same for the config file lookups.
> >
> > > On Nov 30, 2016, at 9:50 AM, Dave Neuman  wrote:
> > >
> > > Hey all,
> > > I have been working on creating some integration tests for the Traffic
> > Ops
> > > client in the psql-rebase branch and fixing any bugs in Traffic Ops
> along
> > > the way.
> > > One thing I would like to change is to have the DeliveryService.create
> > and
> > > Deliveryservice.update in the Traffic Ops API accept cdn name, profile
> > > name, and type name instead of cdn ID, profile ID, and type ID.  I
> > usually
> > > don't like to have clients worry about IDs, I think it should be
> handled
> > on
> > > the server side.  I don't know who all is using the Deliveryservice
> > create
> > > and update APIs, if anyone, so I thought I would make the suggestion
> here
> > > and see if anyone is opposed?
> > > This change would be in a 2.x version of Traffic Ops.
> > >
> > > Thanks,
> > > Dave
> >
>

Re: [jira] [Commented] (TC-28) API response structure should be hierarchical instead of flat

[Jeremy Mitchell]

what i should have done has rolled api/1.3

on monday, i'll get api/1.2 back to where it's always been and move this
new structure into api/1.3

Jeremy

On Fri, Nov 4, 2016 at 3:11 PM, ASF GitHub Bot (JIRA) <j...@apache.org>
wrote:

>
> [ https://issues.apache.org/jira/browse/TC-28?page=com.
> atlassian.jira.plugin.system.issuetabpanels:comment-
> tabpanel=15637694#comment-15637694 ]
>
> ASF GitHub Bot commented on TC-28:
> --
>
> Github user mitchell852 closed the pull request at:
>
> https://github.com/apache/incubator-trafficcontrol/pull/48
>
>
> > API response structure should be hierarchical instead of flat
> > -
> >
> > Key: TC-28
> > URL: https://issues.apache.org/jira/browse/TC-28
> > Project: Traffic Control
> >  Issue Type: Improvement
> >      Components: Traffic Ops API
> >Affects Versions: 1.8.0
> >Reporter: Jeremy Mitchell
> >Priority: Minor
> > Fix For: 1.8.0
> >
> >
> > I created a handful of api endpoints in 1.8 with a flat response
> structure like:
> > {
> > "response": [
> > {
> > "asn": "23",
> > "cachegroupId": "69",
> > "cachegroupName": "Foo_Cachegroup",
> > "id": "60",
> > "lastUpdated": "2016-10-13 12:31:43"
> > }
> > ]
> > }
> > Although this is fine, it makes it more difficult to test when using
> structures derived from the database. This structure is more friendly.
> > {
> > "response": [
> > {
> > "asn": "23",
> > "cachegroup": {
> > "id": "69",
> > "name": "Aberdeen_17802B_Ciscos"
> > },
> > "id": "60",
> > "lastUpdated": "2016-10-13 12:31:43"
> > }
> > ]
> > }
> > This nested structure needs to be applied to api endpoints related to
> asn, cachegroup, deliveryservice, phys_location, region, server and user.
>
>
>
> --
> This message was sent by Atlassian JIRA
> (v6.3.4#6332)
>