You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficcontrol.apache.org by "Gray, Jonathan" <Jo...@comcast.com> on 2019/10/31 22:17:55 UTC
Re: [EXTERNAL] TO API routing blacklist
Not trying to sideswipe, but could we expose that as an endpoint with a Golang list as well to solve: https://github.com/apache/trafficcontrol/issues/2872
Jonathan G
On 10/31/19, 3:51 PM, "Rawlin Peters" <ra...@apache.org> wrote:
Hey all,
We've been picking up momentum in the TO Perl -> Go rewrite recently,
which has gotten me thinking of ways to reduce the risk of possible
regressions in the rewritten APIs. I'd like to propose that we
implement a sort of "routing blacklist" for the TO API routes.
At a high level, this blacklist would be a simple JSON config file like this:
{
"perlRoutes": [
{"method": "GET", "path": "/api/1.1/foos"},
{"method": "POST", "path": "/api/1.1/foos"}
],
"disabledRoutes": [
{"method": "GET", "path": "/api/1.1/foos"},
{"method": "POST", "path": "/api/1.1/foos"}
]
}
APIs in "perlRoutes" would be explicitly routed to the Perl, even if
the Go route exists and could handle the request. The primary use case
for this feature would be to deploy an upgraded version of TO-Go with
rewritten APIs without having TO-Go handle the requests yet.
Post-upgrade, you could remove a rewritten route from the blacklist on
a single TO instance, validate it for some period of time, then roll
out the rewritten API to other TO instances by removing the API from
their blacklists.
APIs in "disabledRoutes" would be explicitly disabled. The use cases
for this field would include:
- disabling endpoints that are part of an incomplete feature and don't
really make sense to use on their own yet
- disabling endpoints that have known, serious issues that should be
disabled immediately
This would make it easier to plug holes in TO without having to
rebuild and redeploy.
Ideally, this config file would be SIGHUP-able so that it can be
reconfigured without having to restart TO. Also, there should be a
hardcoded list of "Perl-routable" routes within TO-Go, so that you
can't just depend on Perl forever if you wanted. I would propose that
newly rewritten routes be added to this hardcoded list in TO-Go. Then
after being in one release as "Perl-routable", they would become
"non-Perl-routable" in the following release.
IMO we could've used something like this since the beginning of the TO
Perl -> Go rewrite effort, but we still do have a decent amount of
routes left that this could be useful for.
Please let me know what you think. If we're generally +1 on this idea,
I'll throw together a blueprint with more details, then maybe I can
convince my boss to let me work on it ;)
- Rawlin
Re: [EXTERNAL] TO API routing blacklist
Posted by Rawlin Peters <ra...@apache.org>.
The IDs for routes wouldn't necessarily need to be documented, since
all of that information would be printed to the log on startup in an
easily digestible format anyways, and that's where you're already at
if you're in the process of disabling/Perling routes.
Making the config use the route regex in the code has the same danger
of invalidating existing configuration, if we ever change the route
regex to fix typos, tighten up the regex, change the path-param names,
etc. Making it ID-based means we'd be free to fix up route regexes in
those cases where necessary.
And like I said before, you'd be free to self-document which routes
you're disabling/Perling in an ignored json field (i.e. a "comment")
in the config. Then you can look at the config file and know which
routes are disabled/Perl'd. I don't like "secret decoder rings" as
much as the next guy, but when the secret decoder ring is right next
to the thing that requires it, I don't see that as much of a problem.
- Rawlin
On Wed, Nov 20, 2019 at 3:24 PM ocket 8888 <oc...@gmail.com> wrote:
>
> I think documenting all of those is going to be more work than using the
> actual paths. I'm also not a fan of numeric IDs in most cases because it
> means
>
> > you wouldn't be able to tell which routes are "Perl'd" or disabled just
> by looking at the config file
>
>
>
> On Wed, Nov 20, 2019 at 3:16 PM Rawlin Peters <ra...@apache.org> wrote:
>
> > No, operators wouldn't need to read the source code, they would just
> > have to read something like this in the log (printed on startup):
> > 1 GET /api/1.1/cdns
> > 2 PUT /api/1.1/cdns
> > ...
> > and so on. Then in cdn.conf you would put:
> > {
> > ....
> > "routing_blacklist": {
> > "perl_routes": [1, 2, 3, 4],
> > "disabled_routes": [5, 6, 7, 8]
> > },
> > ....
> > }
> >
> > The obvious disadvantage would be that you wouldn't be able to tell
> > which routes are "Perl'd" or disabled just by looking at the config
> > file (although you could add comments in ignored json fields if you
> > wanted to), but in practice I don't think this will be much of an
> > issue.
> >
> > Yes, the idea is that the IDs would be statically maintained and not
> > auto-generated on startup (as that could invalidate existing configs
> > as you've said). Basically, we would just have to pick the next unused
> > integer whenever adding a new route. Maintenance cost would be
> > basically zero, since TO would fail to start if you've added a route
> > with an ID that is already taken.
> >
> > - Rawlin
> >
> > On Wed, Nov 20, 2019 at 3:00 PM ocket 8888 <oc...@gmail.com> wrote:
> > >
> > > If you make the configuration ID-based you're telling operators they need
> > > to read the source code to be able to configure TO. Plus if those IDs are
> > > generated sequentially and we need to insert a route in the middle so
> > that
> > > a later rule that would match it doesn't override the route then suddenly
> > > everyone's configuration file is broken. Well, either that or we need to
> > > statically maintain and document a magic number for every API route.
> > >
> > > Idk if this helps at all, but as has been pointed out before the routes
> > > don't actually need to be regular expressions.
> > >
> > > On Wed, Nov 20, 2019 at 2:55 PM Rawlin Peters <ra...@apache.org> wrote:
> > >
> > > > Hey folks,
> > > >
> > > > I'm currently working on this TO API routing blacklist feature, and
> > > > while I've identified which routes should be on the whitelist of
> > > > "routes that have been rewritten to Go but that are still safe to fall
> > > > back to Perl for", the tediousness of copying route regular
> > > > expressions for the whitelist has given me another idea.
> > > >
> > > > Rather than have the configuration be based on regular expressions
> > > > that are meant to match the actual routes in the code, I was thinking
> > > > of giving each route an ID, including the ID as part of the Route
> > > > struct, and making the configuration based on these IDs instead of
> > > > trying to mirror the regex.
> > > >
> > > > That way, you can't accidentally disable routes or have Perl handle
> > > > routes you didn't mean to from writing a bad regular expression.
> > > > Essentially, every route would get a unique ID that can be referenced
> > > > in the config for either disabling it or routing it to Perl. Whether
> > > > or not a Go route would be routable to Perl would also become part of
> > > > the Route struct. TO would print the route IDs on startup (so you can
> > > > easily find them and match them to the routes you're trying to disable
> > > > or fall back to Perl) and verify that the actual given route IDs are
> > > > unique (to ensure that IDs stay unique as routes are moved around or
> > > > new routes are added).
> > > >
> > > > What do you think? Stick to regular expressions, or go with this IDea
> > > > instead (see what I did there)?
> > > >
> > > > - Rawlin
> > > >
> > > > On Fri, Nov 1, 2019 at 4:01 PM Gray, Jonathan <
> > Jonathan_Gray@comcast.com>
> > > > wrote:
> > > > >
> > > > > I largely don't care about the blacklisted routes for this purpose.
> > I
> > > > really care about a conclusive list of whitelisted routes (for which
> > the
> > > > example json payload could be expanded to carry). It seems like we're
> > > > solving the exact same issue from two directions. It permits each
> > native
> > > > client library to assert that the routes it expects and needs to exist,
> > > > exist on the other side. I have no desire to actively modify the
> > runtime
> > > > routes (for security I don't think we every should), just to get the
> > list
> > > > of what it had at startup. Having the override config file on disk to
> > > > switch on/off independent route/methods is something I'd expect to
> > have to
> > > > restart TO for (no different than changes in the cdn.conf). I do also
> > > > agree with proper 503 handling, but it allows us to perform a basic
> > sanity
> > > > check to prevent half-completed workflows necessitating complex
> > recovery
> > > > paths. For applications that use the client SDK, it gives an easy
> > handle
> > > > to know if every single upgrade necessitates recompiling and deploying
> > 3rd
> > > > party applications, such as a CZF File generator.
> > > > >
> > > > > Jonathan G
> > > > >
> > > > >
> > > > > On 11/1/19, 1:49 PM, "Rawlin Peters" <ra...@apache.org> wrote:
> > > > >
> > > > > > Not trying to sideswipe, but could we expose that as an
> > endpoint
> > > > with a Golang list as well to solve:
> > > >
> > https://protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https://github.com/apache/trafficcontrol/issues/2872
> > > > >
> > > > > While I do agree with the request for an API endpoint that tells
> > the
> > > > > client what API versions are supported, I wouldn't want to
> > > > > overcomplicate *this* particular feature with an API endpoint to
> > > > > expose the information that is in the config file.
> > > > >
> > > > > If we implement that kind of "API information" API endpoint, I
> > > > > wouldn't be opposed to including the currently blacklisted
> > routes in
> > > > > its response as a minor goal, but I don't really think it's
> > warranted
> > > > > by this routing blacklist feature alone. You should have a
> > really,
> > > > > really good reason to blacklist a route or bypass a TO-Go route
> > for
> > > > > the Perl, so this should be a (hopefully) relatively rare
> > operation
> > > > to
> > > > > begin with. I don't think it would be all that useful for API
> > clients
> > > > > to be able to see the list of currently blacklisted APIs. The API
> > > > > client should be written to properly handle 503s whenever they
> > occur,
> > > > > and to the client it shouldn't matter if the 503 is from the
> > database
> > > > > being overloaded at the time or if the route is blacklisted.
> > > > >
> > > > > - Rawlin
> > > > >
> > > > >
> > > >
> >
Re: [EXTERNAL] TO API routing blacklist
Posted by Rawlin Peters <ra...@apache.org>.
Part of the issue is that the routes aren't just a simple path. In
order to "whitelist" certain Go routes as "Perl-able", I'd have to
hardcode a list of path regexes that would be checked against given
path regexes in the config, like this:
whitelist[`DELETE ^/api/1\.[1-4]/federations(/|\.json)?$`] = struct{}{}
whitelist[`DELETE
^/api/1\.[1-4]/federations/.+/deliveryservices/.+/?(\.json)?$`] =
struct{}{}
whitelist[`DELETE ^/api/1\.[1-4]/federations/.+/users/.+/?(\.json)?$`]
= struct{}{}
So either way, you'd have to look at _something_ in order to know
which exact regex you have to use in order to configure a Go route to
be handled by Perl. I assumed that printing the hardcoded whitelist to
the app's log would be sufficient, since at that point you could just
copy/paste your desired regex into cdn.conf.
As far as disabling routes, without tying the configuration directly
to the route in the code somehow (via an ID or something else), the
given regexes in the config are just that -- regexes that you assume
match the route you're trying to disable. They may or may not actually
match the route you're trying to disable (if you accidentally messed
up the regex). By taking regexes out of the equation and using IDs
instead, you can know with 100% certainty that if you've effectively
disabled a route, without having to come up with the perfect regex.
Not only that, but you don't have to be an expert in regular
expressions in order to disable routes.
- Rawlin
On Wed, Nov 20, 2019 at 4:04 PM Gray, Jonathan
<Jo...@comcast.com> wrote:
>
> Agreed. All I should have to specify is a list of the route and method. That's easier to investigate than having to crosslist startup logs. Issue 2872 would tell me exactly what to put in there should I need it without looking at source code.
>
> Jonathan G
>
> On 11/20/19, 3:25 PM, "ocket 8888" <oc...@gmail.com> wrote:
>
> I think documenting all of those is going to be more work than using the
> actual paths. I'm also not a fan of numeric IDs in most cases because it
> means
>
> > you wouldn't be able to tell which routes are "Perl'd" or disabled just
> by looking at the config file
>
>
>
> On Wed, Nov 20, 2019 at 3:16 PM Rawlin Peters <ra...@apache.org> wrote:
>
> > No, operators wouldn't need to read the source code, they would just
> > have to read something like this in the log (printed on startup):
> > 1 GET /api/1.1/cdns
> > 2 PUT /api/1.1/cdns
> > ...
> > and so on. Then in cdn.conf you would put:
> > {
> > ....
> > "routing_blacklist": {
> > "perl_routes": [1, 2, 3, 4],
> > "disabled_routes": [5, 6, 7, 8]
> > },
> > ....
> > }
> >
> > The obvious disadvantage would be that you wouldn't be able to tell
> > which routes are "Perl'd" or disabled just by looking at the config
> > file (although you could add comments in ignored json fields if you
> > wanted to), but in practice I don't think this will be much of an
> > issue.
> >
> > Yes, the idea is that the IDs would be statically maintained and not
> > auto-generated on startup (as that could invalidate existing configs
> > as you've said). Basically, we would just have to pick the next unused
> > integer whenever adding a new route. Maintenance cost would be
> > basically zero, since TO would fail to start if you've added a route
> > with an ID that is already taken.
> >
> > - Rawlin
> >
> > On Wed, Nov 20, 2019 at 3:00 PM ocket 8888 <oc...@gmail.com> wrote:
> > >
> > > If you make the configuration ID-based you're telling operators they need
> > > to read the source code to be able to configure TO. Plus if those IDs are
> > > generated sequentially and we need to insert a route in the middle so
> > that
> > > a later rule that would match it doesn't override the route then suddenly
> > > everyone's configuration file is broken. Well, either that or we need to
> > > statically maintain and document a magic number for every API route.
> > >
> > > Idk if this helps at all, but as has been pointed out before the routes
> > > don't actually need to be regular expressions.
> > >
> > > On Wed, Nov 20, 2019 at 2:55 PM Rawlin Peters <ra...@apache.org> wrote:
> > >
> > > > Hey folks,
> > > >
> > > > I'm currently working on this TO API routing blacklist feature, and
> > > > while I've identified which routes should be on the whitelist of
> > > > "routes that have been rewritten to Go but that are still safe to fall
> > > > back to Perl for", the tediousness of copying route regular
> > > > expressions for the whitelist has given me another idea.
> > > >
> > > > Rather than have the configuration be based on regular expressions
> > > > that are meant to match the actual routes in the code, I was thinking
> > > > of giving each route an ID, including the ID as part of the Route
> > > > struct, and making the configuration based on these IDs instead of
> > > > trying to mirror the regex.
> > > >
> > > > That way, you can't accidentally disable routes or have Perl handle
> > > > routes you didn't mean to from writing a bad regular expression.
> > > > Essentially, every route would get a unique ID that can be referenced
> > > > in the config for either disabling it or routing it to Perl. Whether
> > > > or not a Go route would be routable to Perl would also become part of
> > > > the Route struct. TO would print the route IDs on startup (so you can
> > > > easily find them and match them to the routes you're trying to disable
> > > > or fall back to Perl) and verify that the actual given route IDs are
> > > > unique (to ensure that IDs stay unique as routes are moved around or
> > > > new routes are added).
> > > >
> > > > What do you think? Stick to regular expressions, or go with this IDea
> > > > instead (see what I did there)?
> > > >
> > > > - Rawlin
> > > >
> > > > On Fri, Nov 1, 2019 at 4:01 PM Gray, Jonathan <
> > Jonathan_Gray@comcast.com>
> > > > wrote:
> > > > >
> > > > > I largely don't care about the blacklisted routes for this purpose.
> > I
> > > > really care about a conclusive list of whitelisted routes (for which
> > the
> > > > example json payload could be expanded to carry). It seems like we're
> > > > solving the exact same issue from two directions. It permits each
> > native
> > > > client library to assert that the routes it expects and needs to exist,
> > > > exist on the other side. I have no desire to actively modify the
> > runtime
> > > > routes (for security I don't think we every should), just to get the
> > list
> > > > of what it had at startup. Having the override config file on disk to
> > > > switch on/off independent route/methods is something I'd expect to
> > have to
> > > > restart TO for (no different than changes in the cdn.conf). I do also
> > > > agree with proper 503 handling, but it allows us to perform a basic
> > sanity
> > > > check to prevent half-completed workflows necessitating complex
> > recovery
> > > > paths. For applications that use the client SDK, it gives an easy
> > handle
> > > > to know if every single upgrade necessitates recompiling and deploying
> > 3rd
> > > > party applications, such as a CZF File generator.
> > > > >
> > > > > Jonathan G
> > > > >
> > > > >
> > > > > On 11/1/19, 1:49 PM, "Rawlin Peters" <ra...@apache.org> wrote:
> > > > >
> > > > > > Not trying to sideswipe, but could we expose that as an
> > endpoint
> > > > with a Golang list as well to solve:
> > > >
> > https://urldefense.com/v3/__https://protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!TWJwrq9UabYEY283jp9IfCwB33AlGehKAw38xOXA8VShKN51fsJIfLoj149Ld1iEMQMd$
> > > > >
> > > > > While I do agree with the request for an API endpoint that tells
> > the
> > > > > client what API versions are supported, I wouldn't want to
> > > > > overcomplicate *this* particular feature with an API endpoint to
> > > > > expose the information that is in the config file.
> > > > >
> > > > > If we implement that kind of "API information" API endpoint, I
> > > > > wouldn't be opposed to including the currently blacklisted
> > routes in
> > > > > its response as a minor goal, but I don't really think it's
> > warranted
> > > > > by this routing blacklist feature alone. You should have a
> > really,
> > > > > really good reason to blacklist a route or bypass a TO-Go route
> > for
> > > > > the Perl, so this should be a (hopefully) relatively rare
> > operation
> > > > to
> > > > > begin with. I don't think it would be all that useful for API
> > clients
> > > > > to be able to see the list of currently blacklisted APIs. The API
> > > > > client should be written to properly handle 503s whenever they
> > occur,
> > > > > and to the client it shouldn't matter if the 503 is from the
> > database
> > > > > being overloaded at the time or if the route is blacklisted.
> > > > >
> > > > > - Rawlin
> > > > >
> > > > >
> > > >
> >
>
>
Re: [EXTERNAL] TO API routing blacklist
Posted by "Gray, Jonathan" <Jo...@comcast.com>.
Agreed. All I should have to specify is a list of the route and method. That's easier to investigate than having to crosslist startup logs. Issue 2872 would tell me exactly what to put in there should I need it without looking at source code.
Jonathan G
On 11/20/19, 3:25 PM, "ocket 8888" <oc...@gmail.com> wrote:
I think documenting all of those is going to be more work than using the
actual paths. I'm also not a fan of numeric IDs in most cases because it
means
> you wouldn't be able to tell which routes are "Perl'd" or disabled just
by looking at the config file
On Wed, Nov 20, 2019 at 3:16 PM Rawlin Peters <ra...@apache.org> wrote:
> No, operators wouldn't need to read the source code, they would just
> have to read something like this in the log (printed on startup):
> 1 GET /api/1.1/cdns
> 2 PUT /api/1.1/cdns
> ...
> and so on. Then in cdn.conf you would put:
> {
> ....
> "routing_blacklist": {
> "perl_routes": [1, 2, 3, 4],
> "disabled_routes": [5, 6, 7, 8]
> },
> ....
> }
>
> The obvious disadvantage would be that you wouldn't be able to tell
> which routes are "Perl'd" or disabled just by looking at the config
> file (although you could add comments in ignored json fields if you
> wanted to), but in practice I don't think this will be much of an
> issue.
>
> Yes, the idea is that the IDs would be statically maintained and not
> auto-generated on startup (as that could invalidate existing configs
> as you've said). Basically, we would just have to pick the next unused
> integer whenever adding a new route. Maintenance cost would be
> basically zero, since TO would fail to start if you've added a route
> with an ID that is already taken.
>
> - Rawlin
>
> On Wed, Nov 20, 2019 at 3:00 PM ocket 8888 <oc...@gmail.com> wrote:
> >
> > If you make the configuration ID-based you're telling operators they need
> > to read the source code to be able to configure TO. Plus if those IDs are
> > generated sequentially and we need to insert a route in the middle so
> that
> > a later rule that would match it doesn't override the route then suddenly
> > everyone's configuration file is broken. Well, either that or we need to
> > statically maintain and document a magic number for every API route.
> >
> > Idk if this helps at all, but as has been pointed out before the routes
> > don't actually need to be regular expressions.
> >
> > On Wed, Nov 20, 2019 at 2:55 PM Rawlin Peters <ra...@apache.org> wrote:
> >
> > > Hey folks,
> > >
> > > I'm currently working on this TO API routing blacklist feature, and
> > > while I've identified which routes should be on the whitelist of
> > > "routes that have been rewritten to Go but that are still safe to fall
> > > back to Perl for", the tediousness of copying route regular
> > > expressions for the whitelist has given me another idea.
> > >
> > > Rather than have the configuration be based on regular expressions
> > > that are meant to match the actual routes in the code, I was thinking
> > > of giving each route an ID, including the ID as part of the Route
> > > struct, and making the configuration based on these IDs instead of
> > > trying to mirror the regex.
> > >
> > > That way, you can't accidentally disable routes or have Perl handle
> > > routes you didn't mean to from writing a bad regular expression.
> > > Essentially, every route would get a unique ID that can be referenced
> > > in the config for either disabling it or routing it to Perl. Whether
> > > or not a Go route would be routable to Perl would also become part of
> > > the Route struct. TO would print the route IDs on startup (so you can
> > > easily find them and match them to the routes you're trying to disable
> > > or fall back to Perl) and verify that the actual given route IDs are
> > > unique (to ensure that IDs stay unique as routes are moved around or
> > > new routes are added).
> > >
> > > What do you think? Stick to regular expressions, or go with this IDea
> > > instead (see what I did there)?
> > >
> > > - Rawlin
> > >
> > > On Fri, Nov 1, 2019 at 4:01 PM Gray, Jonathan <
> Jonathan_Gray@comcast.com>
> > > wrote:
> > > >
> > > > I largely don't care about the blacklisted routes for this purpose.
> I
> > > really care about a conclusive list of whitelisted routes (for which
> the
> > > example json payload could be expanded to carry). It seems like we're
> > > solving the exact same issue from two directions. It permits each
> native
> > > client library to assert that the routes it expects and needs to exist,
> > > exist on the other side. I have no desire to actively modify the
> runtime
> > > routes (for security I don't think we every should), just to get the
> list
> > > of what it had at startup. Having the override config file on disk to
> > > switch on/off independent route/methods is something I'd expect to
> have to
> > > restart TO for (no different than changes in the cdn.conf). I do also
> > > agree with proper 503 handling, but it allows us to perform a basic
> sanity
> > > check to prevent half-completed workflows necessitating complex
> recovery
> > > paths. For applications that use the client SDK, it gives an easy
> handle
> > > to know if every single upgrade necessitates recompiling and deploying
> 3rd
> > > party applications, such as a CZF File generator.
> > > >
> > > > Jonathan G
> > > >
> > > >
> > > > On 11/1/19, 1:49 PM, "Rawlin Peters" <ra...@apache.org> wrote:
> > > >
> > > > > Not trying to sideswipe, but could we expose that as an
> endpoint
> > > with a Golang list as well to solve:
> > >
> https://urldefense.com/v3/__https://protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!TWJwrq9UabYEY283jp9IfCwB33AlGehKAw38xOXA8VShKN51fsJIfLoj149Ld1iEMQMd$
> > > >
> > > > While I do agree with the request for an API endpoint that tells
> the
> > > > client what API versions are supported, I wouldn't want to
> > > > overcomplicate *this* particular feature with an API endpoint to
> > > > expose the information that is in the config file.
> > > >
> > > > If we implement that kind of "API information" API endpoint, I
> > > > wouldn't be opposed to including the currently blacklisted
> routes in
> > > > its response as a minor goal, but I don't really think it's
> warranted
> > > > by this routing blacklist feature alone. You should have a
> really,
> > > > really good reason to blacklist a route or bypass a TO-Go route
> for
> > > > the Perl, so this should be a (hopefully) relatively rare
> operation
> > > to
> > > > begin with. I don't think it would be all that useful for API
> clients
> > > > to be able to see the list of currently blacklisted APIs. The API
> > > > client should be written to properly handle 503s whenever they
> occur,
> > > > and to the client it shouldn't matter if the 503 is from the
> database
> > > > being overloaded at the time or if the route is blacklisted.
> > > >
> > > > - Rawlin
> > > >
> > > >
> > >
>
Re: [EXTERNAL] TO API routing blacklist
Posted by ocket 8888 <oc...@gmail.com>.
> "... add a CLI option to traffic_ops_golang to just print out the route +
ID table in an easily parseable manner..."
+1 that sounds great
On Thu, Nov 21, 2019 at 9:54 AM Rawlin Peters <ra...@apache.org> wrote:
> Thanks, Neuman. I agree with the points made by Jonathan and
> ocket8888, and if the ID-based solution ends up being a bit of a pain,
> we can always add support for a purely path-based solution later, once
> all the routes are no longer regexes. I just don't want this to be too
> hard for a layman to configure and to avoid the risk of messing up
> regular expressions. Thinking about the startup log issue a bit more,
> it shouldn't be too hard for me to add a CLI option to
> traffic_ops_golang to just print out the route + ID table in an easily
> parseable manner. Hopefully that would alleviate that concern a bit.
>
> - Rawlin
>
> On Thu, Nov 21, 2019 at 8:20 AM Dave Neuman <ne...@apache.org> wrote:
> >
> > First of all I will admit that I did not read every email in this chain.
> > That being said, based on the fact that A) Rawlin is doing this work, B)
> I
> > would like to get this feature into 4.x, and C) I would like 4.x to be
> cut
> > in the next week, I will defer to Rawlin's idea and +1 that. We don't
> need
> > to let perfect be the enemy of good and if something doesn't quite work
> > right and we come up with a better solution later, then we can implement
> it
> > later. This is something that is meant to be short-lived and used for a
> > specific purpose.
> >
> > --Dave
> >
> > On Wed, Nov 20, 2019 at 4:55 PM Rawlin Peters <ra...@apache.org> wrote:
> >
> > > I already have a regex-based implementation working; that is why I am
> > > proposing the ID-based implementation. I saw the light. I looked at
> > > what I had done and thought better. I really think this is a much
> > > safer and easier way to accomplish the same goal.
> > >
> > > While I'm totally on board with making the API routes not regexes, I'm
> > > not going to rewrite all of that existing API route regex
> > > functionality just to be able to use non-regex paths in cdn.conf, for
> > > a feature that is meant to be rarely used in the first place and
> > > already provides the ID -> route mapping in an easy and readily
> > > available manner. We should definitely do that whenever we get around
> > > to API 2.0 (whether it's a _true_ API 2.0 or just a thinly veiled
> > > guise of 1.x without some routes).
> > >
> > > This is feature creep and bike-shedding to the max. We could really
> > > use this feature for 4.0, given that about 70 routes have been
> > > rewritten from Perl to Go and not vetted in a production environment
> > > yet (that I know of... please speak up if you're running master in
> > > Prod).
> > >
> > > - Rawlin
> > >
> > > On Wed, Nov 20, 2019 at 4:48 PM ocket 8888 <oc...@gmail.com>
> wrote:
> > > >
> > > > The routes don't need to be regular expressions though. That's a
> change
> > > > that still needs to be made, and while it would have other benefits
> > > beyond
> > > > this whitelist/blacklist, it is a significant amount of work that
> may or
> > > > may not be more than the work of getting regular expressions to
> work, so
> > > > idk if you wanna do that but it's true. Then, as far as path
> parameters,
> > > > you can just strip them before comparison by doing something like
> > > >
> > > > regex.MustCompile(`\{[^\}]+\}`).Replace(path, "")
> > > >
> > > > not actually familiar with that api, but the point is that
> comparisons
> > > can
> > > > be made to ignore path parameters probably pretty easily - as long as
> > > they
> > > > are just strings.
> > > >
> > > > On Wed, Nov 20, 2019 at 4:31 PM Rawlin Peters <ra...@apache.org>
> wrote:
> > > >
> > > > > That's an interesting idea, but I think that might make this
> feature
> > > > > too complicated, especially with Perl going away. Ultimately,
> clients
> > > > > shouldn't have to worry about which backend is actually handling
> their
> > > > > request, and this feature is mainly about only falling back to the
> > > > > Perl handler in case serious regressions are discovered in the
> > > > > Go-rewritten handler.
> > > > >
> > > > > - Rawlin
> > > > >
> > > > > On Wed, Nov 20, 2019 at 4:10 PM Williams, Adam
> > > > > <Ad...@comcast.com> wrote:
> > > > > >
> > > > > > An alternative implementation could be for the TrafficOps client
> to
> > > be
> > > > > able to specify the backend, on a per request basis (via HTTP
> header,
> > > > > params, or some other mechanism), instead of a whitelist in
> > > TrafficOps. In
> > > > > this case, TrafficOps would still ultimately decide if a route
> could be
> > > > > serviced by multiple backends or not.
> > > > > >
> > > > > > One nice property is that tests, automated or otherwise, could
> easily
> > > > > compare behavior from multiple backends. For example, a test case
> in
> > > the
> > > > > traffic_ops/testing/api/v14 directory could use the same logic to
> test
> > > both
> > > > > the Perl and Go implementations.
> > > > > >
> > > > > > From: ocket 8888 <oc...@gmail.com>
> > > > > > Reply-To: "dev@trafficcontrol.apache.org" <
> > > dev@trafficcontrol.apache.org
> > > > > >
> > > > > > Date: Wednesday, November 20, 2019 at 15:25
> > > > > > To: "dev@trafficcontrol.apache.org" <
> dev@trafficcontrol.apache.org>
> > > > > > Subject: Re: [EXTERNAL] TO API routing blacklist
> > > > > >
> > > > > > I think documenting all of those is going to be more work than
> using
> > > the
> > > > > > actual paths. I'm also not a fan of numeric IDs in most cases
> > > because it
> > > > > > means
> > > > > >
> > > > > > you wouldn't be able to tell which routes are "Perl'd" or
> disabled
> > > just
> > > > > > by looking at the config file
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Wed, Nov 20, 2019 at 3:16 PM Rawlin Peters <rawlin@apache.org
> > > <mailto:
> > > > > rawlin@apache.org>> wrote:
> > > > > >
> > > > > > No, operators wouldn't need to read the source code, they would
> just
> > > > > > have to read something like this in the log (printed on startup):
> > > > > > 1 GET /api/1.1/cdns
> > > > > > 2 PUT /api/1.1/cdns
> > > > > > ...
> > > > > > and so on. Then in cdn.conf you would put:
> > > > > > {
> > > > > > ....
> > > > > > "routing_blacklist": {
> > > > > > "perl_routes": [1, 2, 3, 4],
> > > > > > "disabled_routes": [5, 6, 7, 8]
> > > > > > },
> > > > > > ....
> > > > > > }
> > > > > >
> > > > > > The obvious disadvantage would be that you wouldn't be able to
> tell
> > > > > > which routes are "Perl'd" or disabled just by looking at the
> config
> > > > > > file (although you could add comments in ignored json fields if
> you
> > > > > > wanted to), but in practice I don't think this will be much of an
> > > > > > issue.
> > > > > >
> > > > > > Yes, the idea is that the IDs would be statically maintained and
> not
> > > > > > auto-generated on startup (as that could invalidate existing
> configs
> > > > > > as you've said). Basically, we would just have to pick the next
> > > unused
> > > > > > integer whenever adding a new route. Maintenance cost would be
> > > > > > basically zero, since TO would fail to start if you've added a
> route
> > > > > > with an ID that is already taken.
> > > > > >
> > > > > > - Rawlin
> > > > > >
> > > > > > On Wed, Nov 20, 2019 at 3:00 PM ocket 8888 <ocket8888@gmail.com
> > > <mailto:
> > > > > ocket8888@gmail.com>> wrote:
> > > > > > >
> > > > > > > If you make the configuration ID-based you're telling operators
> > > they
> > > > > need
> > > > > > > to read the source code to be able to configure TO. Plus if
> those
> > > IDs
> > > > > are
> > > > > > > generated sequentially and we need to insert a route in the
> middle
> > > so
> > > > > > that
> > > > > > > a later rule that would match it doesn't override the route
> then
> > > > > suddenly
> > > > > > > everyone's configuration file is broken. Well, either that or
> we
> > > need
> > > > > to
> > > > > > > statically maintain and document a magic number for every API
> > > route.
> > > > > > >
> > > > > > > Idk if this helps at all, but as has been pointed out before
> the
> > > routes
> > > > > > > don't actually need to be regular expressions.
> > > > > > >
> > > > > > > On Wed, Nov 20, 2019 at 2:55 PM Rawlin Peters <
> rawlin@apache.org
> > > > > <ma...@apache.org>> wrote:
> > > > > > >
> > > > > > > > Hey folks,
> > > > > > > >
> > > > > > > > I'm currently working on this TO API routing blacklist
> feature,
> > > and
> > > > > > > > while I've identified which routes should be on the
> whitelist of
> > > > > > > > "routes that have been rewritten to Go but that are still
> safe to
> > > > > fall
> > > > > > > > back to Perl for", the tediousness of copying route regular
> > > > > > > > expressions for the whitelist has given me another idea.
> > > > > > > >
> > > > > > > > Rather than have the configuration be based on regular
> > > expressions
> > > > > > > > that are meant to match the actual routes in the code, I was
> > > thinking
> > > > > > > > of giving each route an ID, including the ID as part of the
> Route
> > > > > > > > struct, and making the configuration based on these IDs
> instead
> > > of
> > > > > > > > trying to mirror the regex.
> > > > > > > >
> > > > > > > > That way, you can't accidentally disable routes or have Perl
> > > handle
> > > > > > > > routes you didn't mean to from writing a bad regular
> expression.
> > > > > > > > Essentially, every route would get a unique ID that can be
> > > referenced
> > > > > > > > in the config for either disabling it or routing it to Perl.
> > > Whether
> > > > > > > > or not a Go route would be routable to Perl would also become
> > > part of
> > > > > > > > the Route struct. TO would print the route IDs on startup (so
> > > you can
> > > > > > > > easily find them and match them to the routes you're trying
> to
> > > > > disable
> > > > > > > > or fall back to Perl) and verify that the actual given route
> IDs
> > > are
> > > > > > > > unique (to ensure that IDs stay unique as routes are moved
> > > around or
> > > > > > > > new routes are added).
> > > > > > > >
> > > > > > > > What do you think? Stick to regular expressions, or go with
> this
> > > IDea
> > > > > > > > instead (see what I did there)?
> > > > > > > >
> > > > > > > > - Rawlin
> > > > > > > >
> > > > > > > > On Fri, Nov 1, 2019 at 4:01 PM Gray, Jonathan <
> > > > > > Jonathan_Gray@comcast.com<ma...@comcast.com>>
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > I largely don't care about the blacklisted routes for this
> > > purpose.
> > > > > > I
> > > > > > > > really care about a conclusive list of whitelisted routes
> (for
> > > which
> > > > > > the
> > > > > > > > example json payload could be expanded to carry). It seems
> like
> > > > > we're
> > > > > > > > solving the exact same issue from two directions. It permits
> > > each
> > > > > > native
> > > > > > > > client library to assert that the routes it expects and
> needs to
> > > > > exist,
> > > > > > > > exist on the other side. I have no desire to actively
> modify the
> > > > > > runtime
> > > > > > > > routes (for security I don't think we every should), just to
> get
> > > the
> > > > > > list
> > > > > > > > of what it had at startup. Having the override config file
> on
> > > disk
> > > > > to
> > > > > > > > switch on/off independent route/methods is something I'd
> expect
> > > to
> > > > > > have to
> > > > > > > > restart TO for (no different than changes in the cdn.conf).
> I do
> > > > > also
> > > > > > > > agree with proper 503 handling, but it allows us to perform a
> > > basic
> > > > > > sanity
> > > > > > > > check to prevent half-completed workflows necessitating
> complex
> > > > > > recovery
> > > > > > > > paths. For applications that use the client SDK, it gives an
> > > easy
> > > > > > handle
> > > > > > > > to know if every single upgrade necessitates recompiling and
> > > > > deploying
> > > > > > 3rd
> > > > > > > > party applications, such as a CZF File generator.
> > > > > > > > >
> > > > > > > > > Jonathan G
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On 11/1/19, 1:49 PM, "Rawlin Peters" <rawlin@apache.org
> > > <mailto:
> > > > > rawlin@apache.org>> wrote:
> > > > > > > > >
> > > > > > > > > > Not trying to sideswipe, but could we expose that as
> an
> > > > > > endpoint
> > > > > > > > with a Golang list as well to solve:
> > > > > > > >
> > > > > >
> > > > >
> > >
> https://urldefense.com/v3/__https://protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!VMUtDsupfJ7TCk0L1Blt_1O4ovxM1nalp21_Fpmbp1Htn9o2oWOC83kc0nWYfOWdDzCc$
> > > > > <
> > > > >
> > >
> https://urldefense.com/v3/__https:/protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!VMUtDsupfJ7TCk0L1Blt_1O4ovxM1nalp21_Fpmbp1Htn9o2oWOC83kc0nWYfOWdDzCc$
> > > > > >
> > > > > > > > >
> > > > > > > > > While I do agree with the request for an API endpoint
> that
> > > > > tells
> > > > > > the
> > > > > > > > > client what API versions are supported, I wouldn't
> want to
> > > > > > > > > overcomplicate *this* particular feature with an API
> > > endpoint
> > > > > to
> > > > > > > > > expose the information that is in the config file.
> > > > > > > > >
> > > > > > > > > If we implement that kind of "API information" API
> > > endpoint, I
> > > > > > > > > wouldn't be opposed to including the currently
> blacklisted
> > > > > > routes in
> > > > > > > > > its response as a minor goal, but I don't really think
> it's
> > > > > > warranted
> > > > > > > > > by this routing blacklist feature alone. You should
> have a
> > > > > > really,
> > > > > > > > > really good reason to blacklist a route or bypass a
> TO-Go
> > > route
> > > > > > for
> > > > > > > > > the Perl, so this should be a (hopefully) relatively
> rare
> > > > > > operation
> > > > > > > > to
> > > > > > > > > begin with. I don't think it would be all that useful
> for
> > > API
> > > > > > clients
> > > > > > > > > to be able to see the list of currently blacklisted
> APIs.
> > > The
> > > > > API
> > > > > > > > > client should be written to properly handle 503s
> whenever
> > > they
> > > > > > occur,
> > > > > > > > > and to the client it shouldn't matter if the 503 is
> from
> > > the
> > > > > > database
> > > > > > > > > being overloaded at the time or if the route is
> > > blacklisted.
> > > > > > > > >
> > > > > > > > > - Rawlin
> > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > >
> > > > > >
> > > > >
> > >
>
Re: [EXTERNAL] TO API routing blacklist
Posted by Rawlin Peters <ra...@apache.org>.
Thanks, Neuman. I agree with the points made by Jonathan and
ocket8888, and if the ID-based solution ends up being a bit of a pain,
we can always add support for a purely path-based solution later, once
all the routes are no longer regexes. I just don't want this to be too
hard for a layman to configure and to avoid the risk of messing up
regular expressions. Thinking about the startup log issue a bit more,
it shouldn't be too hard for me to add a CLI option to
traffic_ops_golang to just print out the route + ID table in an easily
parseable manner. Hopefully that would alleviate that concern a bit.
- Rawlin
On Thu, Nov 21, 2019 at 8:20 AM Dave Neuman <ne...@apache.org> wrote:
>
> First of all I will admit that I did not read every email in this chain.
> That being said, based on the fact that A) Rawlin is doing this work, B) I
> would like to get this feature into 4.x, and C) I would like 4.x to be cut
> in the next week, I will defer to Rawlin's idea and +1 that. We don't need
> to let perfect be the enemy of good and if something doesn't quite work
> right and we come up with a better solution later, then we can implement it
> later. This is something that is meant to be short-lived and used for a
> specific purpose.
>
> --Dave
>
> On Wed, Nov 20, 2019 at 4:55 PM Rawlin Peters <ra...@apache.org> wrote:
>
> > I already have a regex-based implementation working; that is why I am
> > proposing the ID-based implementation. I saw the light. I looked at
> > what I had done and thought better. I really think this is a much
> > safer and easier way to accomplish the same goal.
> >
> > While I'm totally on board with making the API routes not regexes, I'm
> > not going to rewrite all of that existing API route regex
> > functionality just to be able to use non-regex paths in cdn.conf, for
> > a feature that is meant to be rarely used in the first place and
> > already provides the ID -> route mapping in an easy and readily
> > available manner. We should definitely do that whenever we get around
> > to API 2.0 (whether it's a _true_ API 2.0 or just a thinly veiled
> > guise of 1.x without some routes).
> >
> > This is feature creep and bike-shedding to the max. We could really
> > use this feature for 4.0, given that about 70 routes have been
> > rewritten from Perl to Go and not vetted in a production environment
> > yet (that I know of... please speak up if you're running master in
> > Prod).
> >
> > - Rawlin
> >
> > On Wed, Nov 20, 2019 at 4:48 PM ocket 8888 <oc...@gmail.com> wrote:
> > >
> > > The routes don't need to be regular expressions though. That's a change
> > > that still needs to be made, and while it would have other benefits
> > beyond
> > > this whitelist/blacklist, it is a significant amount of work that may or
> > > may not be more than the work of getting regular expressions to work, so
> > > idk if you wanna do that but it's true. Then, as far as path parameters,
> > > you can just strip them before comparison by doing something like
> > >
> > > regex.MustCompile(`\{[^\}]+\}`).Replace(path, "")
> > >
> > > not actually familiar with that api, but the point is that comparisons
> > can
> > > be made to ignore path parameters probably pretty easily - as long as
> > they
> > > are just strings.
> > >
> > > On Wed, Nov 20, 2019 at 4:31 PM Rawlin Peters <ra...@apache.org> wrote:
> > >
> > > > That's an interesting idea, but I think that might make this feature
> > > > too complicated, especially with Perl going away. Ultimately, clients
> > > > shouldn't have to worry about which backend is actually handling their
> > > > request, and this feature is mainly about only falling back to the
> > > > Perl handler in case serious regressions are discovered in the
> > > > Go-rewritten handler.
> > > >
> > > > - Rawlin
> > > >
> > > > On Wed, Nov 20, 2019 at 4:10 PM Williams, Adam
> > > > <Ad...@comcast.com> wrote:
> > > > >
> > > > > An alternative implementation could be for the TrafficOps client to
> > be
> > > > able to specify the backend, on a per request basis (via HTTP header,
> > > > params, or some other mechanism), instead of a whitelist in
> > TrafficOps. In
> > > > this case, TrafficOps would still ultimately decide if a route could be
> > > > serviced by multiple backends or not.
> > > > >
> > > > > One nice property is that tests, automated or otherwise, could easily
> > > > compare behavior from multiple backends. For example, a test case in
> > the
> > > > traffic_ops/testing/api/v14 directory could use the same logic to test
> > both
> > > > the Perl and Go implementations.
> > > > >
> > > > > From: ocket 8888 <oc...@gmail.com>
> > > > > Reply-To: "dev@trafficcontrol.apache.org" <
> > dev@trafficcontrol.apache.org
> > > > >
> > > > > Date: Wednesday, November 20, 2019 at 15:25
> > > > > To: "dev@trafficcontrol.apache.org" <de...@trafficcontrol.apache.org>
> > > > > Subject: Re: [EXTERNAL] TO API routing blacklist
> > > > >
> > > > > I think documenting all of those is going to be more work than using
> > the
> > > > > actual paths. I'm also not a fan of numeric IDs in most cases
> > because it
> > > > > means
> > > > >
> > > > > you wouldn't be able to tell which routes are "Perl'd" or disabled
> > just
> > > > > by looking at the config file
> > > > >
> > > > >
> > > > >
> > > > > On Wed, Nov 20, 2019 at 3:16 PM Rawlin Peters <rawlin@apache.org
> > <mailto:
> > > > rawlin@apache.org>> wrote:
> > > > >
> > > > > No, operators wouldn't need to read the source code, they would just
> > > > > have to read something like this in the log (printed on startup):
> > > > > 1 GET /api/1.1/cdns
> > > > > 2 PUT /api/1.1/cdns
> > > > > ...
> > > > > and so on. Then in cdn.conf you would put:
> > > > > {
> > > > > ....
> > > > > "routing_blacklist": {
> > > > > "perl_routes": [1, 2, 3, 4],
> > > > > "disabled_routes": [5, 6, 7, 8]
> > > > > },
> > > > > ....
> > > > > }
> > > > >
> > > > > The obvious disadvantage would be that you wouldn't be able to tell
> > > > > which routes are "Perl'd" or disabled just by looking at the config
> > > > > file (although you could add comments in ignored json fields if you
> > > > > wanted to), but in practice I don't think this will be much of an
> > > > > issue.
> > > > >
> > > > > Yes, the idea is that the IDs would be statically maintained and not
> > > > > auto-generated on startup (as that could invalidate existing configs
> > > > > as you've said). Basically, we would just have to pick the next
> > unused
> > > > > integer whenever adding a new route. Maintenance cost would be
> > > > > basically zero, since TO would fail to start if you've added a route
> > > > > with an ID that is already taken.
> > > > >
> > > > > - Rawlin
> > > > >
> > > > > On Wed, Nov 20, 2019 at 3:00 PM ocket 8888 <ocket8888@gmail.com
> > <mailto:
> > > > ocket8888@gmail.com>> wrote:
> > > > > >
> > > > > > If you make the configuration ID-based you're telling operators
> > they
> > > > need
> > > > > > to read the source code to be able to configure TO. Plus if those
> > IDs
> > > > are
> > > > > > generated sequentially and we need to insert a route in the middle
> > so
> > > > > that
> > > > > > a later rule that would match it doesn't override the route then
> > > > suddenly
> > > > > > everyone's configuration file is broken. Well, either that or we
> > need
> > > > to
> > > > > > statically maintain and document a magic number for every API
> > route.
> > > > > >
> > > > > > Idk if this helps at all, but as has been pointed out before the
> > routes
> > > > > > don't actually need to be regular expressions.
> > > > > >
> > > > > > On Wed, Nov 20, 2019 at 2:55 PM Rawlin Peters <rawlin@apache.org
> > > > <ma...@apache.org>> wrote:
> > > > > >
> > > > > > > Hey folks,
> > > > > > >
> > > > > > > I'm currently working on this TO API routing blacklist feature,
> > and
> > > > > > > while I've identified which routes should be on the whitelist of
> > > > > > > "routes that have been rewritten to Go but that are still safe to
> > > > fall
> > > > > > > back to Perl for", the tediousness of copying route regular
> > > > > > > expressions for the whitelist has given me another idea.
> > > > > > >
> > > > > > > Rather than have the configuration be based on regular
> > expressions
> > > > > > > that are meant to match the actual routes in the code, I was
> > thinking
> > > > > > > of giving each route an ID, including the ID as part of the Route
> > > > > > > struct, and making the configuration based on these IDs instead
> > of
> > > > > > > trying to mirror the regex.
> > > > > > >
> > > > > > > That way, you can't accidentally disable routes or have Perl
> > handle
> > > > > > > routes you didn't mean to from writing a bad regular expression.
> > > > > > > Essentially, every route would get a unique ID that can be
> > referenced
> > > > > > > in the config for either disabling it or routing it to Perl.
> > Whether
> > > > > > > or not a Go route would be routable to Perl would also become
> > part of
> > > > > > > the Route struct. TO would print the route IDs on startup (so
> > you can
> > > > > > > easily find them and match them to the routes you're trying to
> > > > disable
> > > > > > > or fall back to Perl) and verify that the actual given route IDs
> > are
> > > > > > > unique (to ensure that IDs stay unique as routes are moved
> > around or
> > > > > > > new routes are added).
> > > > > > >
> > > > > > > What do you think? Stick to regular expressions, or go with this
> > IDea
> > > > > > > instead (see what I did there)?
> > > > > > >
> > > > > > > - Rawlin
> > > > > > >
> > > > > > > On Fri, Nov 1, 2019 at 4:01 PM Gray, Jonathan <
> > > > > Jonathan_Gray@comcast.com<ma...@comcast.com>>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > I largely don't care about the blacklisted routes for this
> > purpose.
> > > > > I
> > > > > > > really care about a conclusive list of whitelisted routes (for
> > which
> > > > > the
> > > > > > > example json payload could be expanded to carry). It seems like
> > > > we're
> > > > > > > solving the exact same issue from two directions. It permits
> > each
> > > > > native
> > > > > > > client library to assert that the routes it expects and needs to
> > > > exist,
> > > > > > > exist on the other side. I have no desire to actively modify the
> > > > > runtime
> > > > > > > routes (for security I don't think we every should), just to get
> > the
> > > > > list
> > > > > > > of what it had at startup. Having the override config file on
> > disk
> > > > to
> > > > > > > switch on/off independent route/methods is something I'd expect
> > to
> > > > > have to
> > > > > > > restart TO for (no different than changes in the cdn.conf). I do
> > > > also
> > > > > > > agree with proper 503 handling, but it allows us to perform a
> > basic
> > > > > sanity
> > > > > > > check to prevent half-completed workflows necessitating complex
> > > > > recovery
> > > > > > > paths. For applications that use the client SDK, it gives an
> > easy
> > > > > handle
> > > > > > > to know if every single upgrade necessitates recompiling and
> > > > deploying
> > > > > 3rd
> > > > > > > party applications, such as a CZF File generator.
> > > > > > > >
> > > > > > > > Jonathan G
> > > > > > > >
> > > > > > > >
> > > > > > > > On 11/1/19, 1:49 PM, "Rawlin Peters" <rawlin@apache.org
> > <mailto:
> > > > rawlin@apache.org>> wrote:
> > > > > > > >
> > > > > > > > > Not trying to sideswipe, but could we expose that as an
> > > > > endpoint
> > > > > > > with a Golang list as well to solve:
> > > > > > >
> > > > >
> > > >
> > https://urldefense.com/v3/__https://protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!VMUtDsupfJ7TCk0L1Blt_1O4ovxM1nalp21_Fpmbp1Htn9o2oWOC83kc0nWYfOWdDzCc$
> > > > <
> > > >
> > https://urldefense.com/v3/__https:/protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!VMUtDsupfJ7TCk0L1Blt_1O4ovxM1nalp21_Fpmbp1Htn9o2oWOC83kc0nWYfOWdDzCc$
> > > > >
> > > > > > > >
> > > > > > > > While I do agree with the request for an API endpoint that
> > > > tells
> > > > > the
> > > > > > > > client what API versions are supported, I wouldn't want to
> > > > > > > > overcomplicate *this* particular feature with an API
> > endpoint
> > > > to
> > > > > > > > expose the information that is in the config file.
> > > > > > > >
> > > > > > > > If we implement that kind of "API information" API
> > endpoint, I
> > > > > > > > wouldn't be opposed to including the currently blacklisted
> > > > > routes in
> > > > > > > > its response as a minor goal, but I don't really think it's
> > > > > warranted
> > > > > > > > by this routing blacklist feature alone. You should have a
> > > > > really,
> > > > > > > > really good reason to blacklist a route or bypass a TO-Go
> > route
> > > > > for
> > > > > > > > the Perl, so this should be a (hopefully) relatively rare
> > > > > operation
> > > > > > > to
> > > > > > > > begin with. I don't think it would be all that useful for
> > API
> > > > > clients
> > > > > > > > to be able to see the list of currently blacklisted APIs.
> > The
> > > > API
> > > > > > > > client should be written to properly handle 503s whenever
> > they
> > > > > occur,
> > > > > > > > and to the client it shouldn't matter if the 503 is from
> > the
> > > > > database
> > > > > > > > being overloaded at the time or if the route is
> > blacklisted.
> > > > > > > >
> > > > > > > > - Rawlin
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > >
> > > > >
> > > >
> >
Re: [EXTERNAL] TO API routing blacklist
Posted by ocket 8888 <oc...@gmail.com>.
Yeah, I was gonna say it sounds like Rawlin's already thought about this
from every angle, and I'm certainly not about to jump and change the
routing in the space of an hour so he can move forward with it. I don't
mean to stand in the way of progress, it's just scary to think of things
that are meant to be short-lived and used for a specific purpose winding up
long-standing things that everyone rolls theirs eyes about but can't/won't
change because there are bigger fires and whatnot (e.g. "RASCAL_" mandatory
prefix). Hopefully Perl won't survive the version roll to 5.x.
On Thu, Nov 21, 2019 at 8:20 AM Dave Neuman <ne...@apache.org> wrote:
> First of all I will admit that I did not read every email in this chain.
> That being said, based on the fact that A) Rawlin is doing this work, B) I
> would like to get this feature into 4.x, and C) I would like 4.x to be cut
> in the next week, I will defer to Rawlin's idea and +1 that. We don't need
> to let perfect be the enemy of good and if something doesn't quite work
> right and we come up with a better solution later, then we can implement it
> later. This is something that is meant to be short-lived and used for a
> specific purpose.
>
> --Dave
>
> On Wed, Nov 20, 2019 at 4:55 PM Rawlin Peters <ra...@apache.org> wrote:
>
> > I already have a regex-based implementation working; that is why I am
> > proposing the ID-based implementation. I saw the light. I looked at
> > what I had done and thought better. I really think this is a much
> > safer and easier way to accomplish the same goal.
> >
> > While I'm totally on board with making the API routes not regexes, I'm
> > not going to rewrite all of that existing API route regex
> > functionality just to be able to use non-regex paths in cdn.conf, for
> > a feature that is meant to be rarely used in the first place and
> > already provides the ID -> route mapping in an easy and readily
> > available manner. We should definitely do that whenever we get around
> > to API 2.0 (whether it's a _true_ API 2.0 or just a thinly veiled
> > guise of 1.x without some routes).
> >
> > This is feature creep and bike-shedding to the max. We could really
> > use this feature for 4.0, given that about 70 routes have been
> > rewritten from Perl to Go and not vetted in a production environment
> > yet (that I know of... please speak up if you're running master in
> > Prod).
> >
> > - Rawlin
> >
> > On Wed, Nov 20, 2019 at 4:48 PM ocket 8888 <oc...@gmail.com> wrote:
> > >
> > > The routes don't need to be regular expressions though. That's a change
> > > that still needs to be made, and while it would have other benefits
> > beyond
> > > this whitelist/blacklist, it is a significant amount of work that may
> or
> > > may not be more than the work of getting regular expressions to work,
> so
> > > idk if you wanna do that but it's true. Then, as far as path
> parameters,
> > > you can just strip them before comparison by doing something like
> > >
> > > regex.MustCompile(`\{[^\}]+\}`).Replace(path, "")
> > >
> > > not actually familiar with that api, but the point is that comparisons
> > can
> > > be made to ignore path parameters probably pretty easily - as long as
> > they
> > > are just strings.
> > >
> > > On Wed, Nov 20, 2019 at 4:31 PM Rawlin Peters <ra...@apache.org>
> wrote:
> > >
> > > > That's an interesting idea, but I think that might make this feature
> > > > too complicated, especially with Perl going away. Ultimately, clients
> > > > shouldn't have to worry about which backend is actually handling
> their
> > > > request, and this feature is mainly about only falling back to the
> > > > Perl handler in case serious regressions are discovered in the
> > > > Go-rewritten handler.
> > > >
> > > > - Rawlin
> > > >
> > > > On Wed, Nov 20, 2019 at 4:10 PM Williams, Adam
> > > > <Ad...@comcast.com> wrote:
> > > > >
> > > > > An alternative implementation could be for the TrafficOps client to
> > be
> > > > able to specify the backend, on a per request basis (via HTTP header,
> > > > params, or some other mechanism), instead of a whitelist in
> > TrafficOps. In
> > > > this case, TrafficOps would still ultimately decide if a route could
> be
> > > > serviced by multiple backends or not.
> > > > >
> > > > > One nice property is that tests, automated or otherwise, could
> easily
> > > > compare behavior from multiple backends. For example, a test case in
> > the
> > > > traffic_ops/testing/api/v14 directory could use the same logic to
> test
> > both
> > > > the Perl and Go implementations.
> > > > >
> > > > > From: ocket 8888 <oc...@gmail.com>
> > > > > Reply-To: "dev@trafficcontrol.apache.org" <
> > dev@trafficcontrol.apache.org
> > > > >
> > > > > Date: Wednesday, November 20, 2019 at 15:25
> > > > > To: "dev@trafficcontrol.apache.org" <dev@trafficcontrol.apache.org
> >
> > > > > Subject: Re: [EXTERNAL] TO API routing blacklist
> > > > >
> > > > > I think documenting all of those is going to be more work than
> using
> > the
> > > > > actual paths. I'm also not a fan of numeric IDs in most cases
> > because it
> > > > > means
> > > > >
> > > > > you wouldn't be able to tell which routes are "Perl'd" or disabled
> > just
> > > > > by looking at the config file
> > > > >
> > > > >
> > > > >
> > > > > On Wed, Nov 20, 2019 at 3:16 PM Rawlin Peters <rawlin@apache.org
> > <mailto:
> > > > rawlin@apache.org>> wrote:
> > > > >
> > > > > No, operators wouldn't need to read the source code, they would
> just
> > > > > have to read something like this in the log (printed on startup):
> > > > > 1 GET /api/1.1/cdns
> > > > > 2 PUT /api/1.1/cdns
> > > > > ...
> > > > > and so on. Then in cdn.conf you would put:
> > > > > {
> > > > > ....
> > > > > "routing_blacklist": {
> > > > > "perl_routes": [1, 2, 3, 4],
> > > > > "disabled_routes": [5, 6, 7, 8]
> > > > > },
> > > > > ....
> > > > > }
> > > > >
> > > > > The obvious disadvantage would be that you wouldn't be able to tell
> > > > > which routes are "Perl'd" or disabled just by looking at the config
> > > > > file (although you could add comments in ignored json fields if you
> > > > > wanted to), but in practice I don't think this will be much of an
> > > > > issue.
> > > > >
> > > > > Yes, the idea is that the IDs would be statically maintained and
> not
> > > > > auto-generated on startup (as that could invalidate existing
> configs
> > > > > as you've said). Basically, we would just have to pick the next
> > unused
> > > > > integer whenever adding a new route. Maintenance cost would be
> > > > > basically zero, since TO would fail to start if you've added a
> route
> > > > > with an ID that is already taken.
> > > > >
> > > > > - Rawlin
> > > > >
> > > > > On Wed, Nov 20, 2019 at 3:00 PM ocket 8888 <ocket8888@gmail.com
> > <mailto:
> > > > ocket8888@gmail.com>> wrote:
> > > > > >
> > > > > > If you make the configuration ID-based you're telling operators
> > they
> > > > need
> > > > > > to read the source code to be able to configure TO. Plus if those
> > IDs
> > > > are
> > > > > > generated sequentially and we need to insert a route in the
> middle
> > so
> > > > > that
> > > > > > a later rule that would match it doesn't override the route then
> > > > suddenly
> > > > > > everyone's configuration file is broken. Well, either that or we
> > need
> > > > to
> > > > > > statically maintain and document a magic number for every API
> > route.
> > > > > >
> > > > > > Idk if this helps at all, but as has been pointed out before the
> > routes
> > > > > > don't actually need to be regular expressions.
> > > > > >
> > > > > > On Wed, Nov 20, 2019 at 2:55 PM Rawlin Peters <rawlin@apache.org
> > > > <ma...@apache.org>> wrote:
> > > > > >
> > > > > > > Hey folks,
> > > > > > >
> > > > > > > I'm currently working on this TO API routing blacklist feature,
> > and
> > > > > > > while I've identified which routes should be on the whitelist
> of
> > > > > > > "routes that have been rewritten to Go but that are still safe
> to
> > > > fall
> > > > > > > back to Perl for", the tediousness of copying route regular
> > > > > > > expressions for the whitelist has given me another idea.
> > > > > > >
> > > > > > > Rather than have the configuration be based on regular
> > expressions
> > > > > > > that are meant to match the actual routes in the code, I was
> > thinking
> > > > > > > of giving each route an ID, including the ID as part of the
> Route
> > > > > > > struct, and making the configuration based on these IDs instead
> > of
> > > > > > > trying to mirror the regex.
> > > > > > >
> > > > > > > That way, you can't accidentally disable routes or have Perl
> > handle
> > > > > > > routes you didn't mean to from writing a bad regular
> expression.
> > > > > > > Essentially, every route would get a unique ID that can be
> > referenced
> > > > > > > in the config for either disabling it or routing it to Perl.
> > Whether
> > > > > > > or not a Go route would be routable to Perl would also become
> > part of
> > > > > > > the Route struct. TO would print the route IDs on startup (so
> > you can
> > > > > > > easily find them and match them to the routes you're trying to
> > > > disable
> > > > > > > or fall back to Perl) and verify that the actual given route
> IDs
> > are
> > > > > > > unique (to ensure that IDs stay unique as routes are moved
> > around or
> > > > > > > new routes are added).
> > > > > > >
> > > > > > > What do you think? Stick to regular expressions, or go with
> this
> > IDea
> > > > > > > instead (see what I did there)?
> > > > > > >
> > > > > > > - Rawlin
> > > > > > >
> > > > > > > On Fri, Nov 1, 2019 at 4:01 PM Gray, Jonathan <
> > > > > Jonathan_Gray@comcast.com<ma...@comcast.com>>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > I largely don't care about the blacklisted routes for this
> > purpose.
> > > > > I
> > > > > > > really care about a conclusive list of whitelisted routes (for
> > which
> > > > > the
> > > > > > > example json payload could be expanded to carry). It seems
> like
> > > > we're
> > > > > > > solving the exact same issue from two directions. It permits
> > each
> > > > > native
> > > > > > > client library to assert that the routes it expects and needs
> to
> > > > exist,
> > > > > > > exist on the other side. I have no desire to actively modify
> the
> > > > > runtime
> > > > > > > routes (for security I don't think we every should), just to
> get
> > the
> > > > > list
> > > > > > > of what it had at startup. Having the override config file on
> > disk
> > > > to
> > > > > > > switch on/off independent route/methods is something I'd expect
> > to
> > > > > have to
> > > > > > > restart TO for (no different than changes in the cdn.conf). I
> do
> > > > also
> > > > > > > agree with proper 503 handling, but it allows us to perform a
> > basic
> > > > > sanity
> > > > > > > check to prevent half-completed workflows necessitating complex
> > > > > recovery
> > > > > > > paths. For applications that use the client SDK, it gives an
> > easy
> > > > > handle
> > > > > > > to know if every single upgrade necessitates recompiling and
> > > > deploying
> > > > > 3rd
> > > > > > > party applications, such as a CZF File generator.
> > > > > > > >
> > > > > > > > Jonathan G
> > > > > > > >
> > > > > > > >
> > > > > > > > On 11/1/19, 1:49 PM, "Rawlin Peters" <rawlin@apache.org
> > <mailto:
> > > > rawlin@apache.org>> wrote:
> > > > > > > >
> > > > > > > > > Not trying to sideswipe, but could we expose that as an
> > > > > endpoint
> > > > > > > with a Golang list as well to solve:
> > > > > > >
> > > > >
> > > >
> >
> https://urldefense.com/v3/__https://protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!VMUtDsupfJ7TCk0L1Blt_1O4ovxM1nalp21_Fpmbp1Htn9o2oWOC83kc0nWYfOWdDzCc$
> > > > <
> > > >
> >
> https://urldefense.com/v3/__https:/protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!VMUtDsupfJ7TCk0L1Blt_1O4ovxM1nalp21_Fpmbp1Htn9o2oWOC83kc0nWYfOWdDzCc$
> > > > >
> > > > > > > >
> > > > > > > > While I do agree with the request for an API endpoint
> that
> > > > tells
> > > > > the
> > > > > > > > client what API versions are supported, I wouldn't want
> to
> > > > > > > > overcomplicate *this* particular feature with an API
> > endpoint
> > > > to
> > > > > > > > expose the information that is in the config file.
> > > > > > > >
> > > > > > > > If we implement that kind of "API information" API
> > endpoint, I
> > > > > > > > wouldn't be opposed to including the currently
> blacklisted
> > > > > routes in
> > > > > > > > its response as a minor goal, but I don't really think
> it's
> > > > > warranted
> > > > > > > > by this routing blacklist feature alone. You should have
> a
> > > > > really,
> > > > > > > > really good reason to blacklist a route or bypass a TO-Go
> > route
> > > > > for
> > > > > > > > the Perl, so this should be a (hopefully) relatively rare
> > > > > operation
> > > > > > > to
> > > > > > > > begin with. I don't think it would be all that useful for
> > API
> > > > > clients
> > > > > > > > to be able to see the list of currently blacklisted APIs.
> > The
> > > > API
> > > > > > > > client should be written to properly handle 503s whenever
> > they
> > > > > occur,
> > > > > > > > and to the client it shouldn't matter if the 503 is from
> > the
> > > > > database
> > > > > > > > being overloaded at the time or if the route is
> > blacklisted.
> > > > > > > >
> > > > > > > > - Rawlin
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > >
> > > > >
> > > >
> >
>
Re: [EXTERNAL] TO API routing blacklist
Posted by Dave Neuman <ne...@apache.org>.
First of all I will admit that I did not read every email in this chain.
That being said, based on the fact that A) Rawlin is doing this work, B) I
would like to get this feature into 4.x, and C) I would like 4.x to be cut
in the next week, I will defer to Rawlin's idea and +1 that. We don't need
to let perfect be the enemy of good and if something doesn't quite work
right and we come up with a better solution later, then we can implement it
later. This is something that is meant to be short-lived and used for a
specific purpose.
--Dave
On Wed, Nov 20, 2019 at 4:55 PM Rawlin Peters <ra...@apache.org> wrote:
> I already have a regex-based implementation working; that is why I am
> proposing the ID-based implementation. I saw the light. I looked at
> what I had done and thought better. I really think this is a much
> safer and easier way to accomplish the same goal.
>
> While I'm totally on board with making the API routes not regexes, I'm
> not going to rewrite all of that existing API route regex
> functionality just to be able to use non-regex paths in cdn.conf, for
> a feature that is meant to be rarely used in the first place and
> already provides the ID -> route mapping in an easy and readily
> available manner. We should definitely do that whenever we get around
> to API 2.0 (whether it's a _true_ API 2.0 or just a thinly veiled
> guise of 1.x without some routes).
>
> This is feature creep and bike-shedding to the max. We could really
> use this feature for 4.0, given that about 70 routes have been
> rewritten from Perl to Go and not vetted in a production environment
> yet (that I know of... please speak up if you're running master in
> Prod).
>
> - Rawlin
>
> On Wed, Nov 20, 2019 at 4:48 PM ocket 8888 <oc...@gmail.com> wrote:
> >
> > The routes don't need to be regular expressions though. That's a change
> > that still needs to be made, and while it would have other benefits
> beyond
> > this whitelist/blacklist, it is a significant amount of work that may or
> > may not be more than the work of getting regular expressions to work, so
> > idk if you wanna do that but it's true. Then, as far as path parameters,
> > you can just strip them before comparison by doing something like
> >
> > regex.MustCompile(`\{[^\}]+\}`).Replace(path, "")
> >
> > not actually familiar with that api, but the point is that comparisons
> can
> > be made to ignore path parameters probably pretty easily - as long as
> they
> > are just strings.
> >
> > On Wed, Nov 20, 2019 at 4:31 PM Rawlin Peters <ra...@apache.org> wrote:
> >
> > > That's an interesting idea, but I think that might make this feature
> > > too complicated, especially with Perl going away. Ultimately, clients
> > > shouldn't have to worry about which backend is actually handling their
> > > request, and this feature is mainly about only falling back to the
> > > Perl handler in case serious regressions are discovered in the
> > > Go-rewritten handler.
> > >
> > > - Rawlin
> > >
> > > On Wed, Nov 20, 2019 at 4:10 PM Williams, Adam
> > > <Ad...@comcast.com> wrote:
> > > >
> > > > An alternative implementation could be for the TrafficOps client to
> be
> > > able to specify the backend, on a per request basis (via HTTP header,
> > > params, or some other mechanism), instead of a whitelist in
> TrafficOps. In
> > > this case, TrafficOps would still ultimately decide if a route could be
> > > serviced by multiple backends or not.
> > > >
> > > > One nice property is that tests, automated or otherwise, could easily
> > > compare behavior from multiple backends. For example, a test case in
> the
> > > traffic_ops/testing/api/v14 directory could use the same logic to test
> both
> > > the Perl and Go implementations.
> > > >
> > > > From: ocket 8888 <oc...@gmail.com>
> > > > Reply-To: "dev@trafficcontrol.apache.org" <
> dev@trafficcontrol.apache.org
> > > >
> > > > Date: Wednesday, November 20, 2019 at 15:25
> > > > To: "dev@trafficcontrol.apache.org" <de...@trafficcontrol.apache.org>
> > > > Subject: Re: [EXTERNAL] TO API routing blacklist
> > > >
> > > > I think documenting all of those is going to be more work than using
> the
> > > > actual paths. I'm also not a fan of numeric IDs in most cases
> because it
> > > > means
> > > >
> > > > you wouldn't be able to tell which routes are "Perl'd" or disabled
> just
> > > > by looking at the config file
> > > >
> > > >
> > > >
> > > > On Wed, Nov 20, 2019 at 3:16 PM Rawlin Peters <rawlin@apache.org
> <mailto:
> > > rawlin@apache.org>> wrote:
> > > >
> > > > No, operators wouldn't need to read the source code, they would just
> > > > have to read something like this in the log (printed on startup):
> > > > 1 GET /api/1.1/cdns
> > > > 2 PUT /api/1.1/cdns
> > > > ...
> > > > and so on. Then in cdn.conf you would put:
> > > > {
> > > > ....
> > > > "routing_blacklist": {
> > > > "perl_routes": [1, 2, 3, 4],
> > > > "disabled_routes": [5, 6, 7, 8]
> > > > },
> > > > ....
> > > > }
> > > >
> > > > The obvious disadvantage would be that you wouldn't be able to tell
> > > > which routes are "Perl'd" or disabled just by looking at the config
> > > > file (although you could add comments in ignored json fields if you
> > > > wanted to), but in practice I don't think this will be much of an
> > > > issue.
> > > >
> > > > Yes, the idea is that the IDs would be statically maintained and not
> > > > auto-generated on startup (as that could invalidate existing configs
> > > > as you've said). Basically, we would just have to pick the next
> unused
> > > > integer whenever adding a new route. Maintenance cost would be
> > > > basically zero, since TO would fail to start if you've added a route
> > > > with an ID that is already taken.
> > > >
> > > > - Rawlin
> > > >
> > > > On Wed, Nov 20, 2019 at 3:00 PM ocket 8888 <ocket8888@gmail.com
> <mailto:
> > > ocket8888@gmail.com>> wrote:
> > > > >
> > > > > If you make the configuration ID-based you're telling operators
> they
> > > need
> > > > > to read the source code to be able to configure TO. Plus if those
> IDs
> > > are
> > > > > generated sequentially and we need to insert a route in the middle
> so
> > > > that
> > > > > a later rule that would match it doesn't override the route then
> > > suddenly
> > > > > everyone's configuration file is broken. Well, either that or we
> need
> > > to
> > > > > statically maintain and document a magic number for every API
> route.
> > > > >
> > > > > Idk if this helps at all, but as has been pointed out before the
> routes
> > > > > don't actually need to be regular expressions.
> > > > >
> > > > > On Wed, Nov 20, 2019 at 2:55 PM Rawlin Peters <rawlin@apache.org
> > > <ma...@apache.org>> wrote:
> > > > >
> > > > > > Hey folks,
> > > > > >
> > > > > > I'm currently working on this TO API routing blacklist feature,
> and
> > > > > > while I've identified which routes should be on the whitelist of
> > > > > > "routes that have been rewritten to Go but that are still safe to
> > > fall
> > > > > > back to Perl for", the tediousness of copying route regular
> > > > > > expressions for the whitelist has given me another idea.
> > > > > >
> > > > > > Rather than have the configuration be based on regular
> expressions
> > > > > > that are meant to match the actual routes in the code, I was
> thinking
> > > > > > of giving each route an ID, including the ID as part of the Route
> > > > > > struct, and making the configuration based on these IDs instead
> of
> > > > > > trying to mirror the regex.
> > > > > >
> > > > > > That way, you can't accidentally disable routes or have Perl
> handle
> > > > > > routes you didn't mean to from writing a bad regular expression.
> > > > > > Essentially, every route would get a unique ID that can be
> referenced
> > > > > > in the config for either disabling it or routing it to Perl.
> Whether
> > > > > > or not a Go route would be routable to Perl would also become
> part of
> > > > > > the Route struct. TO would print the route IDs on startup (so
> you can
> > > > > > easily find them and match them to the routes you're trying to
> > > disable
> > > > > > or fall back to Perl) and verify that the actual given route IDs
> are
> > > > > > unique (to ensure that IDs stay unique as routes are moved
> around or
> > > > > > new routes are added).
> > > > > >
> > > > > > What do you think? Stick to regular expressions, or go with this
> IDea
> > > > > > instead (see what I did there)?
> > > > > >
> > > > > > - Rawlin
> > > > > >
> > > > > > On Fri, Nov 1, 2019 at 4:01 PM Gray, Jonathan <
> > > > Jonathan_Gray@comcast.com<ma...@comcast.com>>
> > > > > > wrote:
> > > > > > >
> > > > > > > I largely don't care about the blacklisted routes for this
> purpose.
> > > > I
> > > > > > really care about a conclusive list of whitelisted routes (for
> which
> > > > the
> > > > > > example json payload could be expanded to carry). It seems like
> > > we're
> > > > > > solving the exact same issue from two directions. It permits
> each
> > > > native
> > > > > > client library to assert that the routes it expects and needs to
> > > exist,
> > > > > > exist on the other side. I have no desire to actively modify the
> > > > runtime
> > > > > > routes (for security I don't think we every should), just to get
> the
> > > > list
> > > > > > of what it had at startup. Having the override config file on
> disk
> > > to
> > > > > > switch on/off independent route/methods is something I'd expect
> to
> > > > have to
> > > > > > restart TO for (no different than changes in the cdn.conf). I do
> > > also
> > > > > > agree with proper 503 handling, but it allows us to perform a
> basic
> > > > sanity
> > > > > > check to prevent half-completed workflows necessitating complex
> > > > recovery
> > > > > > paths. For applications that use the client SDK, it gives an
> easy
> > > > handle
> > > > > > to know if every single upgrade necessitates recompiling and
> > > deploying
> > > > 3rd
> > > > > > party applications, such as a CZF File generator.
> > > > > > >
> > > > > > > Jonathan G
> > > > > > >
> > > > > > >
> > > > > > > On 11/1/19, 1:49 PM, "Rawlin Peters" <rawlin@apache.org
> <mailto:
> > > rawlin@apache.org>> wrote:
> > > > > > >
> > > > > > > > Not trying to sideswipe, but could we expose that as an
> > > > endpoint
> > > > > > with a Golang list as well to solve:
> > > > > >
> > > >
> > >
> https://urldefense.com/v3/__https://protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!VMUtDsupfJ7TCk0L1Blt_1O4ovxM1nalp21_Fpmbp1Htn9o2oWOC83kc0nWYfOWdDzCc$
> > > <
> > >
> https://urldefense.com/v3/__https:/protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!VMUtDsupfJ7TCk0L1Blt_1O4ovxM1nalp21_Fpmbp1Htn9o2oWOC83kc0nWYfOWdDzCc$
> > > >
> > > > > > >
> > > > > > > While I do agree with the request for an API endpoint that
> > > tells
> > > > the
> > > > > > > client what API versions are supported, I wouldn't want to
> > > > > > > overcomplicate *this* particular feature with an API
> endpoint
> > > to
> > > > > > > expose the information that is in the config file.
> > > > > > >
> > > > > > > If we implement that kind of "API information" API
> endpoint, I
> > > > > > > wouldn't be opposed to including the currently blacklisted
> > > > routes in
> > > > > > > its response as a minor goal, but I don't really think it's
> > > > warranted
> > > > > > > by this routing blacklist feature alone. You should have a
> > > > really,
> > > > > > > really good reason to blacklist a route or bypass a TO-Go
> route
> > > > for
> > > > > > > the Perl, so this should be a (hopefully) relatively rare
> > > > operation
> > > > > > to
> > > > > > > begin with. I don't think it would be all that useful for
> API
> > > > clients
> > > > > > > to be able to see the list of currently blacklisted APIs.
> The
> > > API
> > > > > > > client should be written to properly handle 503s whenever
> they
> > > > occur,
> > > > > > > and to the client it shouldn't matter if the 503 is from
> the
> > > > database
> > > > > > > being overloaded at the time or if the route is
> blacklisted.
> > > > > > >
> > > > > > > - Rawlin
> > > > > > >
> > > > > > >
> > > > > >
> > > >
> > > >
> > >
>
Re: [EXTERNAL] TO API routing blacklist
Posted by Rawlin Peters <ra...@apache.org>.
I already have a regex-based implementation working; that is why I am
proposing the ID-based implementation. I saw the light. I looked at
what I had done and thought better. I really think this is a much
safer and easier way to accomplish the same goal.
While I'm totally on board with making the API routes not regexes, I'm
not going to rewrite all of that existing API route regex
functionality just to be able to use non-regex paths in cdn.conf, for
a feature that is meant to be rarely used in the first place and
already provides the ID -> route mapping in an easy and readily
available manner. We should definitely do that whenever we get around
to API 2.0 (whether it's a _true_ API 2.0 or just a thinly veiled
guise of 1.x without some routes).
This is feature creep and bike-shedding to the max. We could really
use this feature for 4.0, given that about 70 routes have been
rewritten from Perl to Go and not vetted in a production environment
yet (that I know of... please speak up if you're running master in
Prod).
- Rawlin
On Wed, Nov 20, 2019 at 4:48 PM ocket 8888 <oc...@gmail.com> wrote:
>
> The routes don't need to be regular expressions though. That's a change
> that still needs to be made, and while it would have other benefits beyond
> this whitelist/blacklist, it is a significant amount of work that may or
> may not be more than the work of getting regular expressions to work, so
> idk if you wanna do that but it's true. Then, as far as path parameters,
> you can just strip them before comparison by doing something like
>
> regex.MustCompile(`\{[^\}]+\}`).Replace(path, "")
>
> not actually familiar with that api, but the point is that comparisons can
> be made to ignore path parameters probably pretty easily - as long as they
> are just strings.
>
> On Wed, Nov 20, 2019 at 4:31 PM Rawlin Peters <ra...@apache.org> wrote:
>
> > That's an interesting idea, but I think that might make this feature
> > too complicated, especially with Perl going away. Ultimately, clients
> > shouldn't have to worry about which backend is actually handling their
> > request, and this feature is mainly about only falling back to the
> > Perl handler in case serious regressions are discovered in the
> > Go-rewritten handler.
> >
> > - Rawlin
> >
> > On Wed, Nov 20, 2019 at 4:10 PM Williams, Adam
> > <Ad...@comcast.com> wrote:
> > >
> > > An alternative implementation could be for the TrafficOps client to be
> > able to specify the backend, on a per request basis (via HTTP header,
> > params, or some other mechanism), instead of a whitelist in TrafficOps. In
> > this case, TrafficOps would still ultimately decide if a route could be
> > serviced by multiple backends or not.
> > >
> > > One nice property is that tests, automated or otherwise, could easily
> > compare behavior from multiple backends. For example, a test case in the
> > traffic_ops/testing/api/v14 directory could use the same logic to test both
> > the Perl and Go implementations.
> > >
> > > From: ocket 8888 <oc...@gmail.com>
> > > Reply-To: "dev@trafficcontrol.apache.org" <dev@trafficcontrol.apache.org
> > >
> > > Date: Wednesday, November 20, 2019 at 15:25
> > > To: "dev@trafficcontrol.apache.org" <de...@trafficcontrol.apache.org>
> > > Subject: Re: [EXTERNAL] TO API routing blacklist
> > >
> > > I think documenting all of those is going to be more work than using the
> > > actual paths. I'm also not a fan of numeric IDs in most cases because it
> > > means
> > >
> > > you wouldn't be able to tell which routes are "Perl'd" or disabled just
> > > by looking at the config file
> > >
> > >
> > >
> > > On Wed, Nov 20, 2019 at 3:16 PM Rawlin Peters <rawlin@apache.org<mailto:
> > rawlin@apache.org>> wrote:
> > >
> > > No, operators wouldn't need to read the source code, they would just
> > > have to read something like this in the log (printed on startup):
> > > 1 GET /api/1.1/cdns
> > > 2 PUT /api/1.1/cdns
> > > ...
> > > and so on. Then in cdn.conf you would put:
> > > {
> > > ....
> > > "routing_blacklist": {
> > > "perl_routes": [1, 2, 3, 4],
> > > "disabled_routes": [5, 6, 7, 8]
> > > },
> > > ....
> > > }
> > >
> > > The obvious disadvantage would be that you wouldn't be able to tell
> > > which routes are "Perl'd" or disabled just by looking at the config
> > > file (although you could add comments in ignored json fields if you
> > > wanted to), but in practice I don't think this will be much of an
> > > issue.
> > >
> > > Yes, the idea is that the IDs would be statically maintained and not
> > > auto-generated on startup (as that could invalidate existing configs
> > > as you've said). Basically, we would just have to pick the next unused
> > > integer whenever adding a new route. Maintenance cost would be
> > > basically zero, since TO would fail to start if you've added a route
> > > with an ID that is already taken.
> > >
> > > - Rawlin
> > >
> > > On Wed, Nov 20, 2019 at 3:00 PM ocket 8888 <ocket8888@gmail.com<mailto:
> > ocket8888@gmail.com>> wrote:
> > > >
> > > > If you make the configuration ID-based you're telling operators they
> > need
> > > > to read the source code to be able to configure TO. Plus if those IDs
> > are
> > > > generated sequentially and we need to insert a route in the middle so
> > > that
> > > > a later rule that would match it doesn't override the route then
> > suddenly
> > > > everyone's configuration file is broken. Well, either that or we need
> > to
> > > > statically maintain and document a magic number for every API route.
> > > >
> > > > Idk if this helps at all, but as has been pointed out before the routes
> > > > don't actually need to be regular expressions.
> > > >
> > > > On Wed, Nov 20, 2019 at 2:55 PM Rawlin Peters <rawlin@apache.org
> > <ma...@apache.org>> wrote:
> > > >
> > > > > Hey folks,
> > > > >
> > > > > I'm currently working on this TO API routing blacklist feature, and
> > > > > while I've identified which routes should be on the whitelist of
> > > > > "routes that have been rewritten to Go but that are still safe to
> > fall
> > > > > back to Perl for", the tediousness of copying route regular
> > > > > expressions for the whitelist has given me another idea.
> > > > >
> > > > > Rather than have the configuration be based on regular expressions
> > > > > that are meant to match the actual routes in the code, I was thinking
> > > > > of giving each route an ID, including the ID as part of the Route
> > > > > struct, and making the configuration based on these IDs instead of
> > > > > trying to mirror the regex.
> > > > >
> > > > > That way, you can't accidentally disable routes or have Perl handle
> > > > > routes you didn't mean to from writing a bad regular expression.
> > > > > Essentially, every route would get a unique ID that can be referenced
> > > > > in the config for either disabling it or routing it to Perl. Whether
> > > > > or not a Go route would be routable to Perl would also become part of
> > > > > the Route struct. TO would print the route IDs on startup (so you can
> > > > > easily find them and match them to the routes you're trying to
> > disable
> > > > > or fall back to Perl) and verify that the actual given route IDs are
> > > > > unique (to ensure that IDs stay unique as routes are moved around or
> > > > > new routes are added).
> > > > >
> > > > > What do you think? Stick to regular expressions, or go with this IDea
> > > > > instead (see what I did there)?
> > > > >
> > > > > - Rawlin
> > > > >
> > > > > On Fri, Nov 1, 2019 at 4:01 PM Gray, Jonathan <
> > > Jonathan_Gray@comcast.com<ma...@comcast.com>>
> > > > > wrote:
> > > > > >
> > > > > > I largely don't care about the blacklisted routes for this purpose.
> > > I
> > > > > really care about a conclusive list of whitelisted routes (for which
> > > the
> > > > > example json payload could be expanded to carry). It seems like
> > we're
> > > > > solving the exact same issue from two directions. It permits each
> > > native
> > > > > client library to assert that the routes it expects and needs to
> > exist,
> > > > > exist on the other side. I have no desire to actively modify the
> > > runtime
> > > > > routes (for security I don't think we every should), just to get the
> > > list
> > > > > of what it had at startup. Having the override config file on disk
> > to
> > > > > switch on/off independent route/methods is something I'd expect to
> > > have to
> > > > > restart TO for (no different than changes in the cdn.conf). I do
> > also
> > > > > agree with proper 503 handling, but it allows us to perform a basic
> > > sanity
> > > > > check to prevent half-completed workflows necessitating complex
> > > recovery
> > > > > paths. For applications that use the client SDK, it gives an easy
> > > handle
> > > > > to know if every single upgrade necessitates recompiling and
> > deploying
> > > 3rd
> > > > > party applications, such as a CZF File generator.
> > > > > >
> > > > > > Jonathan G
> > > > > >
> > > > > >
> > > > > > On 11/1/19, 1:49 PM, "Rawlin Peters" <rawlin@apache.org<mailto:
> > rawlin@apache.org>> wrote:
> > > > > >
> > > > > > > Not trying to sideswipe, but could we expose that as an
> > > endpoint
> > > > > with a Golang list as well to solve:
> > > > >
> > >
> > https://urldefense.com/v3/__https://protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!VMUtDsupfJ7TCk0L1Blt_1O4ovxM1nalp21_Fpmbp1Htn9o2oWOC83kc0nWYfOWdDzCc$
> > <
> > https://urldefense.com/v3/__https:/protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!VMUtDsupfJ7TCk0L1Blt_1O4ovxM1nalp21_Fpmbp1Htn9o2oWOC83kc0nWYfOWdDzCc$
> > >
> > > > > >
> > > > > > While I do agree with the request for an API endpoint that
> > tells
> > > the
> > > > > > client what API versions are supported, I wouldn't want to
> > > > > > overcomplicate *this* particular feature with an API endpoint
> > to
> > > > > > expose the information that is in the config file.
> > > > > >
> > > > > > If we implement that kind of "API information" API endpoint, I
> > > > > > wouldn't be opposed to including the currently blacklisted
> > > routes in
> > > > > > its response as a minor goal, but I don't really think it's
> > > warranted
> > > > > > by this routing blacklist feature alone. You should have a
> > > really,
> > > > > > really good reason to blacklist a route or bypass a TO-Go route
> > > for
> > > > > > the Perl, so this should be a (hopefully) relatively rare
> > > operation
> > > > > to
> > > > > > begin with. I don't think it would be all that useful for API
> > > clients
> > > > > > to be able to see the list of currently blacklisted APIs. The
> > API
> > > > > > client should be written to properly handle 503s whenever they
> > > occur,
> > > > > > and to the client it shouldn't matter if the 503 is from the
> > > database
> > > > > > being overloaded at the time or if the route is blacklisted.
> > > > > >
> > > > > > - Rawlin
> > > > > >
> > > > > >
> > > > >
> > >
> > >
> >
Re: [EXTERNAL] TO API routing blacklist
Posted by ocket 8888 <oc...@gmail.com>.
The routes don't need to be regular expressions though. That's a change
that still needs to be made, and while it would have other benefits beyond
this whitelist/blacklist, it is a significant amount of work that may or
may not be more than the work of getting regular expressions to work, so
idk if you wanna do that but it's true. Then, as far as path parameters,
you can just strip them before comparison by doing something like
regex.MustCompile(`\{[^\}]+\}`).Replace(path, "")
not actually familiar with that api, but the point is that comparisons can
be made to ignore path parameters probably pretty easily - as long as they
are just strings.
On Wed, Nov 20, 2019 at 4:31 PM Rawlin Peters <ra...@apache.org> wrote:
> That's an interesting idea, but I think that might make this feature
> too complicated, especially with Perl going away. Ultimately, clients
> shouldn't have to worry about which backend is actually handling their
> request, and this feature is mainly about only falling back to the
> Perl handler in case serious regressions are discovered in the
> Go-rewritten handler.
>
> - Rawlin
>
> On Wed, Nov 20, 2019 at 4:10 PM Williams, Adam
> <Ad...@comcast.com> wrote:
> >
> > An alternative implementation could be for the TrafficOps client to be
> able to specify the backend, on a per request basis (via HTTP header,
> params, or some other mechanism), instead of a whitelist in TrafficOps. In
> this case, TrafficOps would still ultimately decide if a route could be
> serviced by multiple backends or not.
> >
> > One nice property is that tests, automated or otherwise, could easily
> compare behavior from multiple backends. For example, a test case in the
> traffic_ops/testing/api/v14 directory could use the same logic to test both
> the Perl and Go implementations.
> >
> > From: ocket 8888 <oc...@gmail.com>
> > Reply-To: "dev@trafficcontrol.apache.org" <dev@trafficcontrol.apache.org
> >
> > Date: Wednesday, November 20, 2019 at 15:25
> > To: "dev@trafficcontrol.apache.org" <de...@trafficcontrol.apache.org>
> > Subject: Re: [EXTERNAL] TO API routing blacklist
> >
> > I think documenting all of those is going to be more work than using the
> > actual paths. I'm also not a fan of numeric IDs in most cases because it
> > means
> >
> > you wouldn't be able to tell which routes are "Perl'd" or disabled just
> > by looking at the config file
> >
> >
> >
> > On Wed, Nov 20, 2019 at 3:16 PM Rawlin Peters <rawlin@apache.org<mailto:
> rawlin@apache.org>> wrote:
> >
> > No, operators wouldn't need to read the source code, they would just
> > have to read something like this in the log (printed on startup):
> > 1 GET /api/1.1/cdns
> > 2 PUT /api/1.1/cdns
> > ...
> > and so on. Then in cdn.conf you would put:
> > {
> > ....
> > "routing_blacklist": {
> > "perl_routes": [1, 2, 3, 4],
> > "disabled_routes": [5, 6, 7, 8]
> > },
> > ....
> > }
> >
> > The obvious disadvantage would be that you wouldn't be able to tell
> > which routes are "Perl'd" or disabled just by looking at the config
> > file (although you could add comments in ignored json fields if you
> > wanted to), but in practice I don't think this will be much of an
> > issue.
> >
> > Yes, the idea is that the IDs would be statically maintained and not
> > auto-generated on startup (as that could invalidate existing configs
> > as you've said). Basically, we would just have to pick the next unused
> > integer whenever adding a new route. Maintenance cost would be
> > basically zero, since TO would fail to start if you've added a route
> > with an ID that is already taken.
> >
> > - Rawlin
> >
> > On Wed, Nov 20, 2019 at 3:00 PM ocket 8888 <ocket8888@gmail.com<mailto:
> ocket8888@gmail.com>> wrote:
> > >
> > > If you make the configuration ID-based you're telling operators they
> need
> > > to read the source code to be able to configure TO. Plus if those IDs
> are
> > > generated sequentially and we need to insert a route in the middle so
> > that
> > > a later rule that would match it doesn't override the route then
> suddenly
> > > everyone's configuration file is broken. Well, either that or we need
> to
> > > statically maintain and document a magic number for every API route.
> > >
> > > Idk if this helps at all, but as has been pointed out before the routes
> > > don't actually need to be regular expressions.
> > >
> > > On Wed, Nov 20, 2019 at 2:55 PM Rawlin Peters <rawlin@apache.org
> <ma...@apache.org>> wrote:
> > >
> > > > Hey folks,
> > > >
> > > > I'm currently working on this TO API routing blacklist feature, and
> > > > while I've identified which routes should be on the whitelist of
> > > > "routes that have been rewritten to Go but that are still safe to
> fall
> > > > back to Perl for", the tediousness of copying route regular
> > > > expressions for the whitelist has given me another idea.
> > > >
> > > > Rather than have the configuration be based on regular expressions
> > > > that are meant to match the actual routes in the code, I was thinking
> > > > of giving each route an ID, including the ID as part of the Route
> > > > struct, and making the configuration based on these IDs instead of
> > > > trying to mirror the regex.
> > > >
> > > > That way, you can't accidentally disable routes or have Perl handle
> > > > routes you didn't mean to from writing a bad regular expression.
> > > > Essentially, every route would get a unique ID that can be referenced
> > > > in the config for either disabling it or routing it to Perl. Whether
> > > > or not a Go route would be routable to Perl would also become part of
> > > > the Route struct. TO would print the route IDs on startup (so you can
> > > > easily find them and match them to the routes you're trying to
> disable
> > > > or fall back to Perl) and verify that the actual given route IDs are
> > > > unique (to ensure that IDs stay unique as routes are moved around or
> > > > new routes are added).
> > > >
> > > > What do you think? Stick to regular expressions, or go with this IDea
> > > > instead (see what I did there)?
> > > >
> > > > - Rawlin
> > > >
> > > > On Fri, Nov 1, 2019 at 4:01 PM Gray, Jonathan <
> > Jonathan_Gray@comcast.com<ma...@comcast.com>>
> > > > wrote:
> > > > >
> > > > > I largely don't care about the blacklisted routes for this purpose.
> > I
> > > > really care about a conclusive list of whitelisted routes (for which
> > the
> > > > example json payload could be expanded to carry). It seems like
> we're
> > > > solving the exact same issue from two directions. It permits each
> > native
> > > > client library to assert that the routes it expects and needs to
> exist,
> > > > exist on the other side. I have no desire to actively modify the
> > runtime
> > > > routes (for security I don't think we every should), just to get the
> > list
> > > > of what it had at startup. Having the override config file on disk
> to
> > > > switch on/off independent route/methods is something I'd expect to
> > have to
> > > > restart TO for (no different than changes in the cdn.conf). I do
> also
> > > > agree with proper 503 handling, but it allows us to perform a basic
> > sanity
> > > > check to prevent half-completed workflows necessitating complex
> > recovery
> > > > paths. For applications that use the client SDK, it gives an easy
> > handle
> > > > to know if every single upgrade necessitates recompiling and
> deploying
> > 3rd
> > > > party applications, such as a CZF File generator.
> > > > >
> > > > > Jonathan G
> > > > >
> > > > >
> > > > > On 11/1/19, 1:49 PM, "Rawlin Peters" <rawlin@apache.org<mailto:
> rawlin@apache.org>> wrote:
> > > > >
> > > > > > Not trying to sideswipe, but could we expose that as an
> > endpoint
> > > > with a Golang list as well to solve:
> > > >
> >
> https://urldefense.com/v3/__https://protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!VMUtDsupfJ7TCk0L1Blt_1O4ovxM1nalp21_Fpmbp1Htn9o2oWOC83kc0nWYfOWdDzCc$
> <
> https://urldefense.com/v3/__https:/protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!VMUtDsupfJ7TCk0L1Blt_1O4ovxM1nalp21_Fpmbp1Htn9o2oWOC83kc0nWYfOWdDzCc$
> >
> > > > >
> > > > > While I do agree with the request for an API endpoint that
> tells
> > the
> > > > > client what API versions are supported, I wouldn't want to
> > > > > overcomplicate *this* particular feature with an API endpoint
> to
> > > > > expose the information that is in the config file.
> > > > >
> > > > > If we implement that kind of "API information" API endpoint, I
> > > > > wouldn't be opposed to including the currently blacklisted
> > routes in
> > > > > its response as a minor goal, but I don't really think it's
> > warranted
> > > > > by this routing blacklist feature alone. You should have a
> > really,
> > > > > really good reason to blacklist a route or bypass a TO-Go route
> > for
> > > > > the Perl, so this should be a (hopefully) relatively rare
> > operation
> > > > to
> > > > > begin with. I don't think it would be all that useful for API
> > clients
> > > > > to be able to see the list of currently blacklisted APIs. The
> API
> > > > > client should be written to properly handle 503s whenever they
> > occur,
> > > > > and to the client it shouldn't matter if the 503 is from the
> > database
> > > > > being overloaded at the time or if the route is blacklisted.
> > > > >
> > > > > - Rawlin
> > > > >
> > > > >
> > > >
> >
> >
>
Re: [EXTERNAL] TO API routing blacklist
Posted by Rawlin Peters <ra...@apache.org>.
That's an interesting idea, but I think that might make this feature
too complicated, especially with Perl going away. Ultimately, clients
shouldn't have to worry about which backend is actually handling their
request, and this feature is mainly about only falling back to the
Perl handler in case serious regressions are discovered in the
Go-rewritten handler.
- Rawlin
On Wed, Nov 20, 2019 at 4:10 PM Williams, Adam
<Ad...@comcast.com> wrote:
>
> An alternative implementation could be for the TrafficOps client to be able to specify the backend, on a per request basis (via HTTP header, params, or some other mechanism), instead of a whitelist in TrafficOps. In this case, TrafficOps would still ultimately decide if a route could be serviced by multiple backends or not.
>
> One nice property is that tests, automated or otherwise, could easily compare behavior from multiple backends. For example, a test case in the traffic_ops/testing/api/v14 directory could use the same logic to test both the Perl and Go implementations.
>
> From: ocket 8888 <oc...@gmail.com>
> Reply-To: "dev@trafficcontrol.apache.org" <de...@trafficcontrol.apache.org>
> Date: Wednesday, November 20, 2019 at 15:25
> To: "dev@trafficcontrol.apache.org" <de...@trafficcontrol.apache.org>
> Subject: Re: [EXTERNAL] TO API routing blacklist
>
> I think documenting all of those is going to be more work than using the
> actual paths. I'm also not a fan of numeric IDs in most cases because it
> means
>
> you wouldn't be able to tell which routes are "Perl'd" or disabled just
> by looking at the config file
>
>
>
> On Wed, Nov 20, 2019 at 3:16 PM Rawlin Peters <ra...@apache.org>> wrote:
>
> No, operators wouldn't need to read the source code, they would just
> have to read something like this in the log (printed on startup):
> 1 GET /api/1.1/cdns
> 2 PUT /api/1.1/cdns
> ...
> and so on. Then in cdn.conf you would put:
> {
> ....
> "routing_blacklist": {
> "perl_routes": [1, 2, 3, 4],
> "disabled_routes": [5, 6, 7, 8]
> },
> ....
> }
>
> The obvious disadvantage would be that you wouldn't be able to tell
> which routes are "Perl'd" or disabled just by looking at the config
> file (although you could add comments in ignored json fields if you
> wanted to), but in practice I don't think this will be much of an
> issue.
>
> Yes, the idea is that the IDs would be statically maintained and not
> auto-generated on startup (as that could invalidate existing configs
> as you've said). Basically, we would just have to pick the next unused
> integer whenever adding a new route. Maintenance cost would be
> basically zero, since TO would fail to start if you've added a route
> with an ID that is already taken.
>
> - Rawlin
>
> On Wed, Nov 20, 2019 at 3:00 PM ocket 8888 <oc...@gmail.com>> wrote:
> >
> > If you make the configuration ID-based you're telling operators they need
> > to read the source code to be able to configure TO. Plus if those IDs are
> > generated sequentially and we need to insert a route in the middle so
> that
> > a later rule that would match it doesn't override the route then suddenly
> > everyone's configuration file is broken. Well, either that or we need to
> > statically maintain and document a magic number for every API route.
> >
> > Idk if this helps at all, but as has been pointed out before the routes
> > don't actually need to be regular expressions.
> >
> > On Wed, Nov 20, 2019 at 2:55 PM Rawlin Peters <ra...@apache.org>> wrote:
> >
> > > Hey folks,
> > >
> > > I'm currently working on this TO API routing blacklist feature, and
> > > while I've identified which routes should be on the whitelist of
> > > "routes that have been rewritten to Go but that are still safe to fall
> > > back to Perl for", the tediousness of copying route regular
> > > expressions for the whitelist has given me another idea.
> > >
> > > Rather than have the configuration be based on regular expressions
> > > that are meant to match the actual routes in the code, I was thinking
> > > of giving each route an ID, including the ID as part of the Route
> > > struct, and making the configuration based on these IDs instead of
> > > trying to mirror the regex.
> > >
> > > That way, you can't accidentally disable routes or have Perl handle
> > > routes you didn't mean to from writing a bad regular expression.
> > > Essentially, every route would get a unique ID that can be referenced
> > > in the config for either disabling it or routing it to Perl. Whether
> > > or not a Go route would be routable to Perl would also become part of
> > > the Route struct. TO would print the route IDs on startup (so you can
> > > easily find them and match them to the routes you're trying to disable
> > > or fall back to Perl) and verify that the actual given route IDs are
> > > unique (to ensure that IDs stay unique as routes are moved around or
> > > new routes are added).
> > >
> > > What do you think? Stick to regular expressions, or go with this IDea
> > > instead (see what I did there)?
> > >
> > > - Rawlin
> > >
> > > On Fri, Nov 1, 2019 at 4:01 PM Gray, Jonathan <
> Jonathan_Gray@comcast.com<ma...@comcast.com>>
> > > wrote:
> > > >
> > > > I largely don't care about the blacklisted routes for this purpose.
> I
> > > really care about a conclusive list of whitelisted routes (for which
> the
> > > example json payload could be expanded to carry). It seems like we're
> > > solving the exact same issue from two directions. It permits each
> native
> > > client library to assert that the routes it expects and needs to exist,
> > > exist on the other side. I have no desire to actively modify the
> runtime
> > > routes (for security I don't think we every should), just to get the
> list
> > > of what it had at startup. Having the override config file on disk to
> > > switch on/off independent route/methods is something I'd expect to
> have to
> > > restart TO for (no different than changes in the cdn.conf). I do also
> > > agree with proper 503 handling, but it allows us to perform a basic
> sanity
> > > check to prevent half-completed workflows necessitating complex
> recovery
> > > paths. For applications that use the client SDK, it gives an easy
> handle
> > > to know if every single upgrade necessitates recompiling and deploying
> 3rd
> > > party applications, such as a CZF File generator.
> > > >
> > > > Jonathan G
> > > >
> > > >
> > > > On 11/1/19, 1:49 PM, "Rawlin Peters" <ra...@apache.org>> wrote:
> > > >
> > > > > Not trying to sideswipe, but could we expose that as an
> endpoint
> > > with a Golang list as well to solve:
> > >
> https://urldefense.com/v3/__https://protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!VMUtDsupfJ7TCk0L1Blt_1O4ovxM1nalp21_Fpmbp1Htn9o2oWOC83kc0nWYfOWdDzCc$<https://urldefense.com/v3/__https:/protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!VMUtDsupfJ7TCk0L1Blt_1O4ovxM1nalp21_Fpmbp1Htn9o2oWOC83kc0nWYfOWdDzCc$>
> > > >
> > > > While I do agree with the request for an API endpoint that tells
> the
> > > > client what API versions are supported, I wouldn't want to
> > > > overcomplicate *this* particular feature with an API endpoint to
> > > > expose the information that is in the config file.
> > > >
> > > > If we implement that kind of "API information" API endpoint, I
> > > > wouldn't be opposed to including the currently blacklisted
> routes in
> > > > its response as a minor goal, but I don't really think it's
> warranted
> > > > by this routing blacklist feature alone. You should have a
> really,
> > > > really good reason to blacklist a route or bypass a TO-Go route
> for
> > > > the Perl, so this should be a (hopefully) relatively rare
> operation
> > > to
> > > > begin with. I don't think it would be all that useful for API
> clients
> > > > to be able to see the list of currently blacklisted APIs. The API
> > > > client should be written to properly handle 503s whenever they
> occur,
> > > > and to the client it shouldn't matter if the 503 is from the
> database
> > > > being overloaded at the time or if the route is blacklisted.
> > > >
> > > > - Rawlin
> > > >
> > > >
> > >
>
>
Re: [EXTERNAL] TO API routing blacklist
Posted by "Williams, Adam" <Ad...@comcast.com>.
An alternative implementation could be for the TrafficOps client to be able to specify the backend, on a per request basis (via HTTP header, params, or some other mechanism), instead of a whitelist in TrafficOps. In this case, TrafficOps would still ultimately decide if a route could be serviced by multiple backends or not.
One nice property is that tests, automated or otherwise, could easily compare behavior from multiple backends. For example, a test case in the traffic_ops/testing/api/v14 directory could use the same logic to test both the Perl and Go implementations.
From: ocket 8888 <oc...@gmail.com>
Reply-To: "dev@trafficcontrol.apache.org" <de...@trafficcontrol.apache.org>
Date: Wednesday, November 20, 2019 at 15:25
To: "dev@trafficcontrol.apache.org" <de...@trafficcontrol.apache.org>
Subject: Re: [EXTERNAL] TO API routing blacklist
I think documenting all of those is going to be more work than using the
actual paths. I'm also not a fan of numeric IDs in most cases because it
means
you wouldn't be able to tell which routes are "Perl'd" or disabled just
by looking at the config file
On Wed, Nov 20, 2019 at 3:16 PM Rawlin Peters <ra...@apache.org>> wrote:
No, operators wouldn't need to read the source code, they would just
have to read something like this in the log (printed on startup):
1 GET /api/1.1/cdns
2 PUT /api/1.1/cdns
...
and so on. Then in cdn.conf you would put:
{
....
"routing_blacklist": {
"perl_routes": [1, 2, 3, 4],
"disabled_routes": [5, 6, 7, 8]
},
....
}
The obvious disadvantage would be that you wouldn't be able to tell
which routes are "Perl'd" or disabled just by looking at the config
file (although you could add comments in ignored json fields if you
wanted to), but in practice I don't think this will be much of an
issue.
Yes, the idea is that the IDs would be statically maintained and not
auto-generated on startup (as that could invalidate existing configs
as you've said). Basically, we would just have to pick the next unused
integer whenever adding a new route. Maintenance cost would be
basically zero, since TO would fail to start if you've added a route
with an ID that is already taken.
- Rawlin
On Wed, Nov 20, 2019 at 3:00 PM ocket 8888 <oc...@gmail.com>> wrote:
>
> If you make the configuration ID-based you're telling operators they need
> to read the source code to be able to configure TO. Plus if those IDs are
> generated sequentially and we need to insert a route in the middle so
that
> a later rule that would match it doesn't override the route then suddenly
> everyone's configuration file is broken. Well, either that or we need to
> statically maintain and document a magic number for every API route.
>
> Idk if this helps at all, but as has been pointed out before the routes
> don't actually need to be regular expressions.
>
> On Wed, Nov 20, 2019 at 2:55 PM Rawlin Peters <ra...@apache.org>> wrote:
>
> > Hey folks,
> >
> > I'm currently working on this TO API routing blacklist feature, and
> > while I've identified which routes should be on the whitelist of
> > "routes that have been rewritten to Go but that are still safe to fall
> > back to Perl for", the tediousness of copying route regular
> > expressions for the whitelist has given me another idea.
> >
> > Rather than have the configuration be based on regular expressions
> > that are meant to match the actual routes in the code, I was thinking
> > of giving each route an ID, including the ID as part of the Route
> > struct, and making the configuration based on these IDs instead of
> > trying to mirror the regex.
> >
> > That way, you can't accidentally disable routes or have Perl handle
> > routes you didn't mean to from writing a bad regular expression.
> > Essentially, every route would get a unique ID that can be referenced
> > in the config for either disabling it or routing it to Perl. Whether
> > or not a Go route would be routable to Perl would also become part of
> > the Route struct. TO would print the route IDs on startup (so you can
> > easily find them and match them to the routes you're trying to disable
> > or fall back to Perl) and verify that the actual given route IDs are
> > unique (to ensure that IDs stay unique as routes are moved around or
> > new routes are added).
> >
> > What do you think? Stick to regular expressions, or go with this IDea
> > instead (see what I did there)?
> >
> > - Rawlin
> >
> > On Fri, Nov 1, 2019 at 4:01 PM Gray, Jonathan <
Jonathan_Gray@comcast.com<ma...@comcast.com>>
> > wrote:
> > >
> > > I largely don't care about the blacklisted routes for this purpose.
I
> > really care about a conclusive list of whitelisted routes (for which
the
> > example json payload could be expanded to carry). It seems like we're
> > solving the exact same issue from two directions. It permits each
native
> > client library to assert that the routes it expects and needs to exist,
> > exist on the other side. I have no desire to actively modify the
runtime
> > routes (for security I don't think we every should), just to get the
list
> > of what it had at startup. Having the override config file on disk to
> > switch on/off independent route/methods is something I'd expect to
have to
> > restart TO for (no different than changes in the cdn.conf). I do also
> > agree with proper 503 handling, but it allows us to perform a basic
sanity
> > check to prevent half-completed workflows necessitating complex
recovery
> > paths. For applications that use the client SDK, it gives an easy
handle
> > to know if every single upgrade necessitates recompiling and deploying
3rd
> > party applications, such as a CZF File generator.
> > >
> > > Jonathan G
> > >
> > >
> > > On 11/1/19, 1:49 PM, "Rawlin Peters" <ra...@apache.org>> wrote:
> > >
> > > > Not trying to sideswipe, but could we expose that as an
endpoint
> > with a Golang list as well to solve:
> >
https://urldefense.com/v3/__https://protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!VMUtDsupfJ7TCk0L1Blt_1O4ovxM1nalp21_Fpmbp1Htn9o2oWOC83kc0nWYfOWdDzCc$<https://urldefense.com/v3/__https:/protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https:**Agithub.com*apache*trafficcontrol*issues*2872__;Ly8vLy8v!rx_L75ITgOQ!VMUtDsupfJ7TCk0L1Blt_1O4ovxM1nalp21_Fpmbp1Htn9o2oWOC83kc0nWYfOWdDzCc$>
> > >
> > > While I do agree with the request for an API endpoint that tells
the
> > > client what API versions are supported, I wouldn't want to
> > > overcomplicate *this* particular feature with an API endpoint to
> > > expose the information that is in the config file.
> > >
> > > If we implement that kind of "API information" API endpoint, I
> > > wouldn't be opposed to including the currently blacklisted
routes in
> > > its response as a minor goal, but I don't really think it's
warranted
> > > by this routing blacklist feature alone. You should have a
really,
> > > really good reason to blacklist a route or bypass a TO-Go route
for
> > > the Perl, so this should be a (hopefully) relatively rare
operation
> > to
> > > begin with. I don't think it would be all that useful for API
clients
> > > to be able to see the list of currently blacklisted APIs. The API
> > > client should be written to properly handle 503s whenever they
occur,
> > > and to the client it shouldn't matter if the 503 is from the
database
> > > being overloaded at the time or if the route is blacklisted.
> > >
> > > - Rawlin
> > >
> > >
> >
Re: [EXTERNAL] TO API routing blacklist
Posted by ocket 8888 <oc...@gmail.com>.
I think documenting all of those is going to be more work than using the
actual paths. I'm also not a fan of numeric IDs in most cases because it
means
> you wouldn't be able to tell which routes are "Perl'd" or disabled just
by looking at the config file
On Wed, Nov 20, 2019 at 3:16 PM Rawlin Peters <ra...@apache.org> wrote:
> No, operators wouldn't need to read the source code, they would just
> have to read something like this in the log (printed on startup):
> 1 GET /api/1.1/cdns
> 2 PUT /api/1.1/cdns
> ...
> and so on. Then in cdn.conf you would put:
> {
> ....
> "routing_blacklist": {
> "perl_routes": [1, 2, 3, 4],
> "disabled_routes": [5, 6, 7, 8]
> },
> ....
> }
>
> The obvious disadvantage would be that you wouldn't be able to tell
> which routes are "Perl'd" or disabled just by looking at the config
> file (although you could add comments in ignored json fields if you
> wanted to), but in practice I don't think this will be much of an
> issue.
>
> Yes, the idea is that the IDs would be statically maintained and not
> auto-generated on startup (as that could invalidate existing configs
> as you've said). Basically, we would just have to pick the next unused
> integer whenever adding a new route. Maintenance cost would be
> basically zero, since TO would fail to start if you've added a route
> with an ID that is already taken.
>
> - Rawlin
>
> On Wed, Nov 20, 2019 at 3:00 PM ocket 8888 <oc...@gmail.com> wrote:
> >
> > If you make the configuration ID-based you're telling operators they need
> > to read the source code to be able to configure TO. Plus if those IDs are
> > generated sequentially and we need to insert a route in the middle so
> that
> > a later rule that would match it doesn't override the route then suddenly
> > everyone's configuration file is broken. Well, either that or we need to
> > statically maintain and document a magic number for every API route.
> >
> > Idk if this helps at all, but as has been pointed out before the routes
> > don't actually need to be regular expressions.
> >
> > On Wed, Nov 20, 2019 at 2:55 PM Rawlin Peters <ra...@apache.org> wrote:
> >
> > > Hey folks,
> > >
> > > I'm currently working on this TO API routing blacklist feature, and
> > > while I've identified which routes should be on the whitelist of
> > > "routes that have been rewritten to Go but that are still safe to fall
> > > back to Perl for", the tediousness of copying route regular
> > > expressions for the whitelist has given me another idea.
> > >
> > > Rather than have the configuration be based on regular expressions
> > > that are meant to match the actual routes in the code, I was thinking
> > > of giving each route an ID, including the ID as part of the Route
> > > struct, and making the configuration based on these IDs instead of
> > > trying to mirror the regex.
> > >
> > > That way, you can't accidentally disable routes or have Perl handle
> > > routes you didn't mean to from writing a bad regular expression.
> > > Essentially, every route would get a unique ID that can be referenced
> > > in the config for either disabling it or routing it to Perl. Whether
> > > or not a Go route would be routable to Perl would also become part of
> > > the Route struct. TO would print the route IDs on startup (so you can
> > > easily find them and match them to the routes you're trying to disable
> > > or fall back to Perl) and verify that the actual given route IDs are
> > > unique (to ensure that IDs stay unique as routes are moved around or
> > > new routes are added).
> > >
> > > What do you think? Stick to regular expressions, or go with this IDea
> > > instead (see what I did there)?
> > >
> > > - Rawlin
> > >
> > > On Fri, Nov 1, 2019 at 4:01 PM Gray, Jonathan <
> Jonathan_Gray@comcast.com>
> > > wrote:
> > > >
> > > > I largely don't care about the blacklisted routes for this purpose.
> I
> > > really care about a conclusive list of whitelisted routes (for which
> the
> > > example json payload could be expanded to carry). It seems like we're
> > > solving the exact same issue from two directions. It permits each
> native
> > > client library to assert that the routes it expects and needs to exist,
> > > exist on the other side. I have no desire to actively modify the
> runtime
> > > routes (for security I don't think we every should), just to get the
> list
> > > of what it had at startup. Having the override config file on disk to
> > > switch on/off independent route/methods is something I'd expect to
> have to
> > > restart TO for (no different than changes in the cdn.conf). I do also
> > > agree with proper 503 handling, but it allows us to perform a basic
> sanity
> > > check to prevent half-completed workflows necessitating complex
> recovery
> > > paths. For applications that use the client SDK, it gives an easy
> handle
> > > to know if every single upgrade necessitates recompiling and deploying
> 3rd
> > > party applications, such as a CZF File generator.
> > > >
> > > > Jonathan G
> > > >
> > > >
> > > > On 11/1/19, 1:49 PM, "Rawlin Peters" <ra...@apache.org> wrote:
> > > >
> > > > > Not trying to sideswipe, but could we expose that as an
> endpoint
> > > with a Golang list as well to solve:
> > >
> https://protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https://github.com/apache/trafficcontrol/issues/2872
> > > >
> > > > While I do agree with the request for an API endpoint that tells
> the
> > > > client what API versions are supported, I wouldn't want to
> > > > overcomplicate *this* particular feature with an API endpoint to
> > > > expose the information that is in the config file.
> > > >
> > > > If we implement that kind of "API information" API endpoint, I
> > > > wouldn't be opposed to including the currently blacklisted
> routes in
> > > > its response as a minor goal, but I don't really think it's
> warranted
> > > > by this routing blacklist feature alone. You should have a
> really,
> > > > really good reason to blacklist a route or bypass a TO-Go route
> for
> > > > the Perl, so this should be a (hopefully) relatively rare
> operation
> > > to
> > > > begin with. I don't think it would be all that useful for API
> clients
> > > > to be able to see the list of currently blacklisted APIs. The API
> > > > client should be written to properly handle 503s whenever they
> occur,
> > > > and to the client it shouldn't matter if the 503 is from the
> database
> > > > being overloaded at the time or if the route is blacklisted.
> > > >
> > > > - Rawlin
> > > >
> > > >
> > >
>
Re: [EXTERNAL] TO API routing blacklist
Posted by Rawlin Peters <ra...@apache.org>.
No, operators wouldn't need to read the source code, they would just
have to read something like this in the log (printed on startup):
1 GET /api/1.1/cdns
2 PUT /api/1.1/cdns
...
and so on. Then in cdn.conf you would put:
{
....
"routing_blacklist": {
"perl_routes": [1, 2, 3, 4],
"disabled_routes": [5, 6, 7, 8]
},
....
}
The obvious disadvantage would be that you wouldn't be able to tell
which routes are "Perl'd" or disabled just by looking at the config
file (although you could add comments in ignored json fields if you
wanted to), but in practice I don't think this will be much of an
issue.
Yes, the idea is that the IDs would be statically maintained and not
auto-generated on startup (as that could invalidate existing configs
as you've said). Basically, we would just have to pick the next unused
integer whenever adding a new route. Maintenance cost would be
basically zero, since TO would fail to start if you've added a route
with an ID that is already taken.
- Rawlin
On Wed, Nov 20, 2019 at 3:00 PM ocket 8888 <oc...@gmail.com> wrote:
>
> If you make the configuration ID-based you're telling operators they need
> to read the source code to be able to configure TO. Plus if those IDs are
> generated sequentially and we need to insert a route in the middle so that
> a later rule that would match it doesn't override the route then suddenly
> everyone's configuration file is broken. Well, either that or we need to
> statically maintain and document a magic number for every API route.
>
> Idk if this helps at all, but as has been pointed out before the routes
> don't actually need to be regular expressions.
>
> On Wed, Nov 20, 2019 at 2:55 PM Rawlin Peters <ra...@apache.org> wrote:
>
> > Hey folks,
> >
> > I'm currently working on this TO API routing blacklist feature, and
> > while I've identified which routes should be on the whitelist of
> > "routes that have been rewritten to Go but that are still safe to fall
> > back to Perl for", the tediousness of copying route regular
> > expressions for the whitelist has given me another idea.
> >
> > Rather than have the configuration be based on regular expressions
> > that are meant to match the actual routes in the code, I was thinking
> > of giving each route an ID, including the ID as part of the Route
> > struct, and making the configuration based on these IDs instead of
> > trying to mirror the regex.
> >
> > That way, you can't accidentally disable routes or have Perl handle
> > routes you didn't mean to from writing a bad regular expression.
> > Essentially, every route would get a unique ID that can be referenced
> > in the config for either disabling it or routing it to Perl. Whether
> > or not a Go route would be routable to Perl would also become part of
> > the Route struct. TO would print the route IDs on startup (so you can
> > easily find them and match them to the routes you're trying to disable
> > or fall back to Perl) and verify that the actual given route IDs are
> > unique (to ensure that IDs stay unique as routes are moved around or
> > new routes are added).
> >
> > What do you think? Stick to regular expressions, or go with this IDea
> > instead (see what I did there)?
> >
> > - Rawlin
> >
> > On Fri, Nov 1, 2019 at 4:01 PM Gray, Jonathan <Jo...@comcast.com>
> > wrote:
> > >
> > > I largely don't care about the blacklisted routes for this purpose. I
> > really care about a conclusive list of whitelisted routes (for which the
> > example json payload could be expanded to carry). It seems like we're
> > solving the exact same issue from two directions. It permits each native
> > client library to assert that the routes it expects and needs to exist,
> > exist on the other side. I have no desire to actively modify the runtime
> > routes (for security I don't think we every should), just to get the list
> > of what it had at startup. Having the override config file on disk to
> > switch on/off independent route/methods is something I'd expect to have to
> > restart TO for (no different than changes in the cdn.conf). I do also
> > agree with proper 503 handling, but it allows us to perform a basic sanity
> > check to prevent half-completed workflows necessitating complex recovery
> > paths. For applications that use the client SDK, it gives an easy handle
> > to know if every single upgrade necessitates recompiling and deploying 3rd
> > party applications, such as a CZF File generator.
> > >
> > > Jonathan G
> > >
> > >
> > > On 11/1/19, 1:49 PM, "Rawlin Peters" <ra...@apache.org> wrote:
> > >
> > > > Not trying to sideswipe, but could we expose that as an endpoint
> > with a Golang list as well to solve:
> > https://protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https://github.com/apache/trafficcontrol/issues/2872
> > >
> > > While I do agree with the request for an API endpoint that tells the
> > > client what API versions are supported, I wouldn't want to
> > > overcomplicate *this* particular feature with an API endpoint to
> > > expose the information that is in the config file.
> > >
> > > If we implement that kind of "API information" API endpoint, I
> > > wouldn't be opposed to including the currently blacklisted routes in
> > > its response as a minor goal, but I don't really think it's warranted
> > > by this routing blacklist feature alone. You should have a really,
> > > really good reason to blacklist a route or bypass a TO-Go route for
> > > the Perl, so this should be a (hopefully) relatively rare operation
> > to
> > > begin with. I don't think it would be all that useful for API clients
> > > to be able to see the list of currently blacklisted APIs. The API
> > > client should be written to properly handle 503s whenever they occur,
> > > and to the client it shouldn't matter if the 503 is from the database
> > > being overloaded at the time or if the route is blacklisted.
> > >
> > > - Rawlin
> > >
> > >
> >
Re: [EXTERNAL] TO API routing blacklist
Posted by ocket 8888 <oc...@gmail.com>.
If you make the configuration ID-based you're telling operators they need
to read the source code to be able to configure TO. Plus if those IDs are
generated sequentially and we need to insert a route in the middle so that
a later rule that would match it doesn't override the route then suddenly
everyone's configuration file is broken. Well, either that or we need to
statically maintain and document a magic number for every API route.
Idk if this helps at all, but as has been pointed out before the routes
don't actually need to be regular expressions.
On Wed, Nov 20, 2019 at 2:55 PM Rawlin Peters <ra...@apache.org> wrote:
> Hey folks,
>
> I'm currently working on this TO API routing blacklist feature, and
> while I've identified which routes should be on the whitelist of
> "routes that have been rewritten to Go but that are still safe to fall
> back to Perl for", the tediousness of copying route regular
> expressions for the whitelist has given me another idea.
>
> Rather than have the configuration be based on regular expressions
> that are meant to match the actual routes in the code, I was thinking
> of giving each route an ID, including the ID as part of the Route
> struct, and making the configuration based on these IDs instead of
> trying to mirror the regex.
>
> That way, you can't accidentally disable routes or have Perl handle
> routes you didn't mean to from writing a bad regular expression.
> Essentially, every route would get a unique ID that can be referenced
> in the config for either disabling it or routing it to Perl. Whether
> or not a Go route would be routable to Perl would also become part of
> the Route struct. TO would print the route IDs on startup (so you can
> easily find them and match them to the routes you're trying to disable
> or fall back to Perl) and verify that the actual given route IDs are
> unique (to ensure that IDs stay unique as routes are moved around or
> new routes are added).
>
> What do you think? Stick to regular expressions, or go with this IDea
> instead (see what I did there)?
>
> - Rawlin
>
> On Fri, Nov 1, 2019 at 4:01 PM Gray, Jonathan <Jo...@comcast.com>
> wrote:
> >
> > I largely don't care about the blacklisted routes for this purpose. I
> really care about a conclusive list of whitelisted routes (for which the
> example json payload could be expanded to carry). It seems like we're
> solving the exact same issue from two directions. It permits each native
> client library to assert that the routes it expects and needs to exist,
> exist on the other side. I have no desire to actively modify the runtime
> routes (for security I don't think we every should), just to get the list
> of what it had at startup. Having the override config file on disk to
> switch on/off independent route/methods is something I'd expect to have to
> restart TO for (no different than changes in the cdn.conf). I do also
> agree with proper 503 handling, but it allows us to perform a basic sanity
> check to prevent half-completed workflows necessitating complex recovery
> paths. For applications that use the client SDK, it gives an easy handle
> to know if every single upgrade necessitates recompiling and deploying 3rd
> party applications, such as a CZF File generator.
> >
> > Jonathan G
> >
> >
> > On 11/1/19, 1:49 PM, "Rawlin Peters" <ra...@apache.org> wrote:
> >
> > > Not trying to sideswipe, but could we expose that as an endpoint
> with a Golang list as well to solve:
> https://protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https://github.com/apache/trafficcontrol/issues/2872
> >
> > While I do agree with the request for an API endpoint that tells the
> > client what API versions are supported, I wouldn't want to
> > overcomplicate *this* particular feature with an API endpoint to
> > expose the information that is in the config file.
> >
> > If we implement that kind of "API information" API endpoint, I
> > wouldn't be opposed to including the currently blacklisted routes in
> > its response as a minor goal, but I don't really think it's warranted
> > by this routing blacklist feature alone. You should have a really,
> > really good reason to blacklist a route or bypass a TO-Go route for
> > the Perl, so this should be a (hopefully) relatively rare operation
> to
> > begin with. I don't think it would be all that useful for API clients
> > to be able to see the list of currently blacklisted APIs. The API
> > client should be written to properly handle 503s whenever they occur,
> > and to the client it shouldn't matter if the 503 is from the database
> > being overloaded at the time or if the route is blacklisted.
> >
> > - Rawlin
> >
> >
>
Re: [EXTERNAL] TO API routing blacklist
Posted by Rawlin Peters <ra...@apache.org>.
Hey folks,
I'm currently working on this TO API routing blacklist feature, and
while I've identified which routes should be on the whitelist of
"routes that have been rewritten to Go but that are still safe to fall
back to Perl for", the tediousness of copying route regular
expressions for the whitelist has given me another idea.
Rather than have the configuration be based on regular expressions
that are meant to match the actual routes in the code, I was thinking
of giving each route an ID, including the ID as part of the Route
struct, and making the configuration based on these IDs instead of
trying to mirror the regex.
That way, you can't accidentally disable routes or have Perl handle
routes you didn't mean to from writing a bad regular expression.
Essentially, every route would get a unique ID that can be referenced
in the config for either disabling it or routing it to Perl. Whether
or not a Go route would be routable to Perl would also become part of
the Route struct. TO would print the route IDs on startup (so you can
easily find them and match them to the routes you're trying to disable
or fall back to Perl) and verify that the actual given route IDs are
unique (to ensure that IDs stay unique as routes are moved around or
new routes are added).
What do you think? Stick to regular expressions, or go with this IDea
instead (see what I did there)?
- Rawlin
On Fri, Nov 1, 2019 at 4:01 PM Gray, Jonathan <Jo...@comcast.com> wrote:
>
> I largely don't care about the blacklisted routes for this purpose. I really care about a conclusive list of whitelisted routes (for which the example json payload could be expanded to carry). It seems like we're solving the exact same issue from two directions. It permits each native client library to assert that the routes it expects and needs to exist, exist on the other side. I have no desire to actively modify the runtime routes (for security I don't think we every should), just to get the list of what it had at startup. Having the override config file on disk to switch on/off independent route/methods is something I'd expect to have to restart TO for (no different than changes in the cdn.conf). I do also agree with proper 503 handling, but it allows us to perform a basic sanity check to prevent half-completed workflows necessitating complex recovery paths. For applications that use the client SDK, it gives an easy handle to know if every single upgrade necessitates recompiling and deploying 3rd party applications, such as a CZF File generator.
>
> Jonathan G
>
>
> On 11/1/19, 1:49 PM, "Rawlin Peters" <ra...@apache.org> wrote:
>
> > Not trying to sideswipe, but could we expose that as an endpoint with a Golang list as well to solve: https://protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https://github.com/apache/trafficcontrol/issues/2872
>
> While I do agree with the request for an API endpoint that tells the
> client what API versions are supported, I wouldn't want to
> overcomplicate *this* particular feature with an API endpoint to
> expose the information that is in the config file.
>
> If we implement that kind of "API information" API endpoint, I
> wouldn't be opposed to including the currently blacklisted routes in
> its response as a minor goal, but I don't really think it's warranted
> by this routing blacklist feature alone. You should have a really,
> really good reason to blacklist a route or bypass a TO-Go route for
> the Perl, so this should be a (hopefully) relatively rare operation to
> begin with. I don't think it would be all that useful for API clients
> to be able to see the list of currently blacklisted APIs. The API
> client should be written to properly handle 503s whenever they occur,
> and to the client it shouldn't matter if the 503 is from the database
> being overloaded at the time or if the route is blacklisted.
>
> - Rawlin
>
>
Re: [EXTERNAL] TO API routing blacklist
Posted by Rawlin Peters <ra...@apache.org>.
whoops, my previous reply was to the wrong email thread (meant for the
Release Process thread instead). I'll re-reply to the correct thread
:)
- Rawlin
On Mon, Nov 4, 2019 at 2:40 PM Rawlin Peters <ra...@apache.org> wrote:
>
> > I'm +1 on this final summary. It infers that we are going to soon be adding a job that runs CI checks on Master. This is good. That's probably a subject for another thread. I do have one question about this approach. I think there is probably a scenario where we need to fix an issue on the 4.x branch itself. In this case I guess we would merge that commit back to master? And then RM would have to keep track of when its safe to fast-forward to the merged version of the fix?
>
> My understanding is that we shouldn't ever have a reason to fix an
> issue directly on the 4.x branch itself. When 4.x is ready for a
> release, we will cut a new branch (e.g. 4.1.x) off of 4.x at that
> point. If we find we absolutely need a fix for 4.1.x, then ideally we
> should develop the fix on master then cherry-pick it into 4.1.x. I'm
> not sure we'd also cherry-pick it into 4.x since then 4.x would no
> longer be a snapshot of the most recent stable commit of master
> (meaning we wouldn't be able to just "fast-forward" -- in git
> terminology -- to the next most recent stable commit of master).
>
> IMO unless an absolutely necessary fix is also super time-sensitive,
> we should develop the fix on master and cherry-pick it into the 4.1.x
> branch. Then all cherry-picks would be tracked the same way, without
> also having to track hotfixes that need cherry-picked to master.
> Whenever cherry-picks/hotfixes are used, the important thing is just
> to make sure they get carried forward into the next release (either by
> bringing them in naturally via moving forward on master, or
> re-cherry-picking them). Again, these should only be things we deem
> absolutely necessary for a release (show-stoppers).
>
> - Rawlin
Re: [EXTERNAL] TO API routing blacklist
Posted by Rawlin Peters <ra...@apache.org>.
> I'm +1 on this final summary. It infers that we are going to soon be adding a job that runs CI checks on Master. This is good. That's probably a subject for another thread. I do have one question about this approach. I think there is probably a scenario where we need to fix an issue on the 4.x branch itself. In this case I guess we would merge that commit back to master? And then RM would have to keep track of when its safe to fast-forward to the merged version of the fix?
My understanding is that we shouldn't ever have a reason to fix an
issue directly on the 4.x branch itself. When 4.x is ready for a
release, we will cut a new branch (e.g. 4.1.x) off of 4.x at that
point. If we find we absolutely need a fix for 4.1.x, then ideally we
should develop the fix on master then cherry-pick it into 4.1.x. I'm
not sure we'd also cherry-pick it into 4.x since then 4.x would no
longer be a snapshot of the most recent stable commit of master
(meaning we wouldn't be able to just "fast-forward" -- in git
terminology -- to the next most recent stable commit of master).
IMO unless an absolutely necessary fix is also super time-sensitive,
we should develop the fix on master and cherry-pick it into the 4.1.x
branch. Then all cherry-picks would be tracked the same way, without
also having to track hotfixes that need cherry-picked to master.
Whenever cherry-picks/hotfixes are used, the important thing is just
to make sure they get carried forward into the next release (either by
bringing them in naturally via moving forward on master, or
re-cherry-picking them). Again, these should only be things we deem
absolutely necessary for a release (show-stoppers).
- Rawlin
Re: [EXTERNAL] TO API routing blacklist
Posted by "Gray, Jonathan" <Jo...@comcast.com>.
I largely don't care about the blacklisted routes for this purpose. I really care about a conclusive list of whitelisted routes (for which the example json payload could be expanded to carry). It seems like we're solving the exact same issue from two directions. It permits each native client library to assert that the routes it expects and needs to exist, exist on the other side. I have no desire to actively modify the runtime routes (for security I don't think we every should), just to get the list of what it had at startup. Having the override config file on disk to switch on/off independent route/methods is something I'd expect to have to restart TO for (no different than changes in the cdn.conf). I do also agree with proper 503 handling, but it allows us to perform a basic sanity check to prevent half-completed workflows necessitating complex recovery paths. For applications that use the client SDK, it gives an easy handle to know if every single upgrade necessitates recompiling and deploying 3rd party applications, such as a CZF File generator.
Jonathan G
On 11/1/19, 1:49 PM, "Rawlin Peters" <ra...@apache.org> wrote:
> Not trying to sideswipe, but could we expose that as an endpoint with a Golang list as well to solve: https://protect2.fireeye.com/url?k=5d57b4c9-01b3ba02-5d57937d-000babff3540-17d7cedf2908de8b&u=https://github.com/apache/trafficcontrol/issues/2872
While I do agree with the request for an API endpoint that tells the
client what API versions are supported, I wouldn't want to
overcomplicate *this* particular feature with an API endpoint to
expose the information that is in the config file.
If we implement that kind of "API information" API endpoint, I
wouldn't be opposed to including the currently blacklisted routes in
its response as a minor goal, but I don't really think it's warranted
by this routing blacklist feature alone. You should have a really,
really good reason to blacklist a route or bypass a TO-Go route for
the Perl, so this should be a (hopefully) relatively rare operation to
begin with. I don't think it would be all that useful for API clients
to be able to see the list of currently blacklisted APIs. The API
client should be written to properly handle 503s whenever they occur,
and to the client it shouldn't matter if the 503 is from the database
being overloaded at the time or if the route is blacklisted.
- Rawlin
Re: [EXTERNAL] TO API routing blacklist
Posted by Rawlin Peters <ra...@apache.org>.
> Not trying to sideswipe, but could we expose that as an endpoint with a Golang list as well to solve: https://github.com/apache/trafficcontrol/issues/2872
While I do agree with the request for an API endpoint that tells the
client what API versions are supported, I wouldn't want to
overcomplicate *this* particular feature with an API endpoint to
expose the information that is in the config file.
If we implement that kind of "API information" API endpoint, I
wouldn't be opposed to including the currently blacklisted routes in
its response as a minor goal, but I don't really think it's warranted
by this routing blacklist feature alone. You should have a really,
really good reason to blacklist a route or bypass a TO-Go route for
the Perl, so this should be a (hopefully) relatively rare operation to
begin with. I don't think it would be all that useful for API clients
to be able to see the list of currently blacklisted APIs. The API
client should be written to properly handle 503s whenever they occur,
and to the client it shouldn't matter if the 503 is from the database
being overloaded at the time or if the route is blacklisted.
- Rawlin
Re: [EXTERNAL] TO API routing blacklist
Posted by Rawlin Peters <ra...@apache.org>.
I agree with being vigilant with regards to the Perl. I am still on
board with forcing the Perl route handlers to return a 501 after being
rewritten, but we shouldn't do that in the same release as its
corresponding Go-rewritten route so that we'd still be able to
fallback easily to the Perl handler. That, and we should have a really
good reason to deviate the Go-rewritten handler's behavior from the
Perl in such a way that they wouldn't remain interchangeable. That
should be a conscious decision made by the contributor/community, and
I'd hope that we'd be well aware at that point to know not to keep it
"Perl-routable".
- Rawlin
On Fri, Nov 1, 2019 at 11:45 AM Robert Butts <ro...@apache.org> wrote:
>
> > I would propose that newly rewritten routes be added to this hardcoded
> list in TO-Go. Then after being in one release as "Perl-routable", they
> would become "non-Perl-routable" in the following release.
>
> I'm a big +1, as long as we're extra-vigilant about this. More than
> unroutable, we need to delete the code. The old Perl which has diverged
> from Go is extremely dangerous and can result in catastrophic data loss and
> unpredictable behavior. We got consensus to delete it and change the
> endpoints (which shouldn't be callable) to return a 501, like a year ago,
> but never did it. Which is my fault as much as anyone; but we need to do it.
>
> Also, if we ever change behavior at the same time as a rewrite, we need to
> be careful _not_ to add it to the whitelist, for the reason above, because
> then calling the Perl can result in unexpected behavior and data loss.
>
> But as long as we're careful about that, and don't allow falling back to
> Perl where it's changed, and start deleting the dangerous old code; this is
> a great feature, to not have to roll back a deploy for a single broken
> endpoint. +1
>
>
> On Fri, Nov 1, 2019 at 11:41 AM Jeremy Mitchell <mi...@gmail.com>
> wrote:
>
> > I'm +1 on this idea as well. Not too concerned about the implementation but
> > just having an easy way to "turn off" rewritten Go api routes and fall back
> > to the proven perl routes seems like a nice safety valve. In the past,
> > we've seen bugs creep in to the api rewrite and a quick off switch seems
> > very valuable to have
> >
> > On Fri, Nov 1, 2019 at 11:03 AM Dave Neuman <ne...@apache.org> wrote:
> >
> > > I am +1 on this idea.
> > >
> > > On Fri, Nov 1, 2019 at 09:26 Gray, Jonathan <Jo...@comcast.com>
> > > wrote:
> > >
> > > > The available routes and what associated backend they go to are
> > operator+
> > > > sensitive information. I'd also lean towards a stronger degree of
> > safety
> > > > and only make it read-only.
> > > >
> > > > Jonathan G
> > > >
> > > > On 11/1/19, 8:41 AM, "Hoppal, Michael" <Mi...@comcast.com>
> > > > wrote:
> > > >
> > > > I would be +1 on TO API routing blacklist.
> > > >
> > > > I think it is a great idea to allow us to continue to move fast
> > while
> > > > minimizing risk.
> > > >
> > > > I do agree we should expose the current state of the blacklist as
> > an
> > > > API endpoint as to easily get insight into the current configuration.
> > We
> > > > could even make the blacklist driven by the API instead of a JSON
> > config
> > > > file.
> > > >
> > > > Michael
> > > >
> > > > On 10/31/19, 4:18 PM, "Gray, Jonathan" <Jo...@comcast.com>
> > > > wrote:
> > > >
> > > > Not trying to sideswipe, but could we expose that as an
> > endpoint
> > > > with a Golang list as well to solve:
> > > >
> > >
> > https://protect2.fireeye.com/url?k=1978833f-459c8df4-1978a48b-000babff3540-c717b11942d8382e&u=https://github.com/apache/trafficcontrol/issues/2872
> > > >
> > > > Jonathan G
> > > >
> > > > On 10/31/19, 3:51 PM, "Rawlin Peters" <ra...@apache.org>
> > wrote:
> > > >
> > > > Hey all,
> > > >
> > > > We've been picking up momentum in the TO Perl -> Go rewrite
> > > > recently,
> > > > which has gotten me thinking of ways to reduce the risk of
> > > > possible
> > > > regressions in the rewritten APIs. I'd like to propose that
> > > we
> > > > implement a sort of "routing blacklist" for the TO API
> > > routes.
> > > >
> > > > At a high level, this blacklist would be a simple JSON
> > config
> > > > file like this:
> > > > {
> > > > "perlRoutes": [
> > > > {"method": "GET", "path": "/api/1.1/foos"},
> > > > {"method": "POST", "path": "/api/1.1/foos"}
> > > > ],
> > > > "disabledRoutes": [
> > > > {"method": "GET", "path": "/api/1.1/foos"},
> > > > {"method": "POST", "path": "/api/1.1/foos"}
> > > > ]
> > > > }
> > > >
> > > > APIs in "perlRoutes" would be explicitly routed to the
> > Perl,
> > > > even if
> > > > the Go route exists and could handle the request. The
> > primary
> > > > use case
> > > > for this feature would be to deploy an upgraded version of
> > > > TO-Go with
> > > > rewritten APIs without having TO-Go handle the requests
> > yet.
> > > > Post-upgrade, you could remove a rewritten route from the
> > > > blacklist on
> > > > a single TO instance, validate it for some period of time,
> > > > then roll
> > > > out the rewritten API to other TO instances by removing the
> > > > API from
> > > > their blacklists.
> > > >
> > > > APIs in "disabledRoutes" would be explicitly disabled. The
> > > use
> > > > cases
> > > > for this field would include:
> > > > - disabling endpoints that are part of an incomplete
> > feature
> > > > and don't
> > > > really make sense to use on their own yet
> > > > - disabling endpoints that have known, serious issues that
> > > > should be
> > > > disabled immediately
> > > > This would make it easier to plug holes in TO without
> > having
> > > to
> > > > rebuild and redeploy.
> > > >
> > > > Ideally, this config file would be SIGHUP-able so that it
> > can
> > > > be
> > > > reconfigured without having to restart TO. Also, there
> > should
> > > > be a
> > > > hardcoded list of "Perl-routable" routes within TO-Go, so
> > > that
> > > > you
> > > > can't just depend on Perl forever if you wanted. I would
> > > > propose that
> > > > newly rewritten routes be added to this hardcoded list in
> > > > TO-Go. Then
> > > > after being in one release as "Perl-routable", they would
> > > > become
> > > > "non-Perl-routable" in the following release.
> > > >
> > > > IMO we could've used something like this since the
> > beginning
> > > > of the TO
> > > > Perl -> Go rewrite effort, but we still do have a decent
> > > > amount of
> > > > routes left that this could be useful for.
> > > >
> > > > Please let me know what you think. If we're generally +1 on
> > > > this idea,
> > > > I'll throw together a blueprint with more details, then
> > maybe
> > > > I can
> > > > convince my boss to let me work on it ;)
> > > >
> > > > - Rawlin
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> >
Re: [EXTERNAL] TO API routing blacklist
Posted by Robert Butts <ro...@apache.org>.
> I would propose that newly rewritten routes be added to this hardcoded
list in TO-Go. Then after being in one release as "Perl-routable", they
would become "non-Perl-routable" in the following release.
I'm a big +1, as long as we're extra-vigilant about this. More than
unroutable, we need to delete the code. The old Perl which has diverged
from Go is extremely dangerous and can result in catastrophic data loss and
unpredictable behavior. We got consensus to delete it and change the
endpoints (which shouldn't be callable) to return a 501, like a year ago,
but never did it. Which is my fault as much as anyone; but we need to do it.
Also, if we ever change behavior at the same time as a rewrite, we need to
be careful _not_ to add it to the whitelist, for the reason above, because
then calling the Perl can result in unexpected behavior and data loss.
But as long as we're careful about that, and don't allow falling back to
Perl where it's changed, and start deleting the dangerous old code; this is
a great feature, to not have to roll back a deploy for a single broken
endpoint. +1
On Fri, Nov 1, 2019 at 11:41 AM Jeremy Mitchell <mi...@gmail.com>
wrote:
> I'm +1 on this idea as well. Not too concerned about the implementation but
> just having an easy way to "turn off" rewritten Go api routes and fall back
> to the proven perl routes seems like a nice safety valve. In the past,
> we've seen bugs creep in to the api rewrite and a quick off switch seems
> very valuable to have
>
> On Fri, Nov 1, 2019 at 11:03 AM Dave Neuman <ne...@apache.org> wrote:
>
> > I am +1 on this idea.
> >
> > On Fri, Nov 1, 2019 at 09:26 Gray, Jonathan <Jo...@comcast.com>
> > wrote:
> >
> > > The available routes and what associated backend they go to are
> operator+
> > > sensitive information. I'd also lean towards a stronger degree of
> safety
> > > and only make it read-only.
> > >
> > > Jonathan G
> > >
> > > On 11/1/19, 8:41 AM, "Hoppal, Michael" <Mi...@comcast.com>
> > > wrote:
> > >
> > > I would be +1 on TO API routing blacklist.
> > >
> > > I think it is a great idea to allow us to continue to move fast
> while
> > > minimizing risk.
> > >
> > > I do agree we should expose the current state of the blacklist as
> an
> > > API endpoint as to easily get insight into the current configuration.
> We
> > > could even make the blacklist driven by the API instead of a JSON
> config
> > > file.
> > >
> > > Michael
> > >
> > > On 10/31/19, 4:18 PM, "Gray, Jonathan" <Jo...@comcast.com>
> > > wrote:
> > >
> > > Not trying to sideswipe, but could we expose that as an
> endpoint
> > > with a Golang list as well to solve:
> > >
> >
> https://protect2.fireeye.com/url?k=1978833f-459c8df4-1978a48b-000babff3540-c717b11942d8382e&u=https://github.com/apache/trafficcontrol/issues/2872
> > >
> > > Jonathan G
> > >
> > > On 10/31/19, 3:51 PM, "Rawlin Peters" <ra...@apache.org>
> wrote:
> > >
> > > Hey all,
> > >
> > > We've been picking up momentum in the TO Perl -> Go rewrite
> > > recently,
> > > which has gotten me thinking of ways to reduce the risk of
> > > possible
> > > regressions in the rewritten APIs. I'd like to propose that
> > we
> > > implement a sort of "routing blacklist" for the TO API
> > routes.
> > >
> > > At a high level, this blacklist would be a simple JSON
> config
> > > file like this:
> > > {
> > > "perlRoutes": [
> > > {"method": "GET", "path": "/api/1.1/foos"},
> > > {"method": "POST", "path": "/api/1.1/foos"}
> > > ],
> > > "disabledRoutes": [
> > > {"method": "GET", "path": "/api/1.1/foos"},
> > > {"method": "POST", "path": "/api/1.1/foos"}
> > > ]
> > > }
> > >
> > > APIs in "perlRoutes" would be explicitly routed to the
> Perl,
> > > even if
> > > the Go route exists and could handle the request. The
> primary
> > > use case
> > > for this feature would be to deploy an upgraded version of
> > > TO-Go with
> > > rewritten APIs without having TO-Go handle the requests
> yet.
> > > Post-upgrade, you could remove a rewritten route from the
> > > blacklist on
> > > a single TO instance, validate it for some period of time,
> > > then roll
> > > out the rewritten API to other TO instances by removing the
> > > API from
> > > their blacklists.
> > >
> > > APIs in "disabledRoutes" would be explicitly disabled. The
> > use
> > > cases
> > > for this field would include:
> > > - disabling endpoints that are part of an incomplete
> feature
> > > and don't
> > > really make sense to use on their own yet
> > > - disabling endpoints that have known, serious issues that
> > > should be
> > > disabled immediately
> > > This would make it easier to plug holes in TO without
> having
> > to
> > > rebuild and redeploy.
> > >
> > > Ideally, this config file would be SIGHUP-able so that it
> can
> > > be
> > > reconfigured without having to restart TO. Also, there
> should
> > > be a
> > > hardcoded list of "Perl-routable" routes within TO-Go, so
> > that
> > > you
> > > can't just depend on Perl forever if you wanted. I would
> > > propose that
> > > newly rewritten routes be added to this hardcoded list in
> > > TO-Go. Then
> > > after being in one release as "Perl-routable", they would
> > > become
> > > "non-Perl-routable" in the following release.
> > >
> > > IMO we could've used something like this since the
> beginning
> > > of the TO
> > > Perl -> Go rewrite effort, but we still do have a decent
> > > amount of
> > > routes left that this could be useful for.
> > >
> > > Please let me know what you think. If we're generally +1 on
> > > this idea,
> > > I'll throw together a blueprint with more details, then
> maybe
> > > I can
> > > convince my boss to let me work on it ;)
> > >
> > > - Rawlin
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> >
>
Re: [EXTERNAL] TO API routing blacklist
Posted by Jeremy Mitchell <mi...@gmail.com>.
I'm +1 on this idea as well. Not too concerned about the implementation but
just having an easy way to "turn off" rewritten Go api routes and fall back
to the proven perl routes seems like a nice safety valve. In the past,
we've seen bugs creep in to the api rewrite and a quick off switch seems
very valuable to have
On Fri, Nov 1, 2019 at 11:03 AM Dave Neuman <ne...@apache.org> wrote:
> I am +1 on this idea.
>
> On Fri, Nov 1, 2019 at 09:26 Gray, Jonathan <Jo...@comcast.com>
> wrote:
>
> > The available routes and what associated backend they go to are operator+
> > sensitive information. I'd also lean towards a stronger degree of safety
> > and only make it read-only.
> >
> > Jonathan G
> >
> > On 11/1/19, 8:41 AM, "Hoppal, Michael" <Mi...@comcast.com>
> > wrote:
> >
> > I would be +1 on TO API routing blacklist.
> >
> > I think it is a great idea to allow us to continue to move fast while
> > minimizing risk.
> >
> > I do agree we should expose the current state of the blacklist as an
> > API endpoint as to easily get insight into the current configuration. We
> > could even make the blacklist driven by the API instead of a JSON config
> > file.
> >
> > Michael
> >
> > On 10/31/19, 4:18 PM, "Gray, Jonathan" <Jo...@comcast.com>
> > wrote:
> >
> > Not trying to sideswipe, but could we expose that as an endpoint
> > with a Golang list as well to solve:
> >
> https://protect2.fireeye.com/url?k=1978833f-459c8df4-1978a48b-000babff3540-c717b11942d8382e&u=https://github.com/apache/trafficcontrol/issues/2872
> >
> > Jonathan G
> >
> > On 10/31/19, 3:51 PM, "Rawlin Peters" <ra...@apache.org> wrote:
> >
> > Hey all,
> >
> > We've been picking up momentum in the TO Perl -> Go rewrite
> > recently,
> > which has gotten me thinking of ways to reduce the risk of
> > possible
> > regressions in the rewritten APIs. I'd like to propose that
> we
> > implement a sort of "routing blacklist" for the TO API
> routes.
> >
> > At a high level, this blacklist would be a simple JSON config
> > file like this:
> > {
> > "perlRoutes": [
> > {"method": "GET", "path": "/api/1.1/foos"},
> > {"method": "POST", "path": "/api/1.1/foos"}
> > ],
> > "disabledRoutes": [
> > {"method": "GET", "path": "/api/1.1/foos"},
> > {"method": "POST", "path": "/api/1.1/foos"}
> > ]
> > }
> >
> > APIs in "perlRoutes" would be explicitly routed to the Perl,
> > even if
> > the Go route exists and could handle the request. The primary
> > use case
> > for this feature would be to deploy an upgraded version of
> > TO-Go with
> > rewritten APIs without having TO-Go handle the requests yet.
> > Post-upgrade, you could remove a rewritten route from the
> > blacklist on
> > a single TO instance, validate it for some period of time,
> > then roll
> > out the rewritten API to other TO instances by removing the
> > API from
> > their blacklists.
> >
> > APIs in "disabledRoutes" would be explicitly disabled. The
> use
> > cases
> > for this field would include:
> > - disabling endpoints that are part of an incomplete feature
> > and don't
> > really make sense to use on their own yet
> > - disabling endpoints that have known, serious issues that
> > should be
> > disabled immediately
> > This would make it easier to plug holes in TO without having
> to
> > rebuild and redeploy.
> >
> > Ideally, this config file would be SIGHUP-able so that it can
> > be
> > reconfigured without having to restart TO. Also, there should
> > be a
> > hardcoded list of "Perl-routable" routes within TO-Go, so
> that
> > you
> > can't just depend on Perl forever if you wanted. I would
> > propose that
> > newly rewritten routes be added to this hardcoded list in
> > TO-Go. Then
> > after being in one release as "Perl-routable", they would
> > become
> > "non-Perl-routable" in the following release.
> >
> > IMO we could've used something like this since the beginning
> > of the TO
> > Perl -> Go rewrite effort, but we still do have a decent
> > amount of
> > routes left that this could be useful for.
> >
> > Please let me know what you think. If we're generally +1 on
> > this idea,
> > I'll throw together a blueprint with more details, then maybe
> > I can
> > convince my boss to let me work on it ;)
> >
> > - Rawlin
> >
> >
> >
> >
> >
> >
> >
>
Re: [EXTERNAL] TO API routing blacklist
Posted by Dave Neuman <ne...@apache.org>.
I am +1 on this idea.
On Fri, Nov 1, 2019 at 09:26 Gray, Jonathan <Jo...@comcast.com>
wrote:
> The available routes and what associated backend they go to are operator+
> sensitive information. I'd also lean towards a stronger degree of safety
> and only make it read-only.
>
> Jonathan G
>
> On 11/1/19, 8:41 AM, "Hoppal, Michael" <Mi...@comcast.com>
> wrote:
>
> I would be +1 on TO API routing blacklist.
>
> I think it is a great idea to allow us to continue to move fast while
> minimizing risk.
>
> I do agree we should expose the current state of the blacklist as an
> API endpoint as to easily get insight into the current configuration. We
> could even make the blacklist driven by the API instead of a JSON config
> file.
>
> Michael
>
> On 10/31/19, 4:18 PM, "Gray, Jonathan" <Jo...@comcast.com>
> wrote:
>
> Not trying to sideswipe, but could we expose that as an endpoint
> with a Golang list as well to solve:
> https://protect2.fireeye.com/url?k=1978833f-459c8df4-1978a48b-000babff3540-c717b11942d8382e&u=https://github.com/apache/trafficcontrol/issues/2872
>
> Jonathan G
>
> On 10/31/19, 3:51 PM, "Rawlin Peters" <ra...@apache.org> wrote:
>
> Hey all,
>
> We've been picking up momentum in the TO Perl -> Go rewrite
> recently,
> which has gotten me thinking of ways to reduce the risk of
> possible
> regressions in the rewritten APIs. I'd like to propose that we
> implement a sort of "routing blacklist" for the TO API routes.
>
> At a high level, this blacklist would be a simple JSON config
> file like this:
> {
> "perlRoutes": [
> {"method": "GET", "path": "/api/1.1/foos"},
> {"method": "POST", "path": "/api/1.1/foos"}
> ],
> "disabledRoutes": [
> {"method": "GET", "path": "/api/1.1/foos"},
> {"method": "POST", "path": "/api/1.1/foos"}
> ]
> }
>
> APIs in "perlRoutes" would be explicitly routed to the Perl,
> even if
> the Go route exists and could handle the request. The primary
> use case
> for this feature would be to deploy an upgraded version of
> TO-Go with
> rewritten APIs without having TO-Go handle the requests yet.
> Post-upgrade, you could remove a rewritten route from the
> blacklist on
> a single TO instance, validate it for some period of time,
> then roll
> out the rewritten API to other TO instances by removing the
> API from
> their blacklists.
>
> APIs in "disabledRoutes" would be explicitly disabled. The use
> cases
> for this field would include:
> - disabling endpoints that are part of an incomplete feature
> and don't
> really make sense to use on their own yet
> - disabling endpoints that have known, serious issues that
> should be
> disabled immediately
> This would make it easier to plug holes in TO without having to
> rebuild and redeploy.
>
> Ideally, this config file would be SIGHUP-able so that it can
> be
> reconfigured without having to restart TO. Also, there should
> be a
> hardcoded list of "Perl-routable" routes within TO-Go, so that
> you
> can't just depend on Perl forever if you wanted. I would
> propose that
> newly rewritten routes be added to this hardcoded list in
> TO-Go. Then
> after being in one release as "Perl-routable", they would
> become
> "non-Perl-routable" in the following release.
>
> IMO we could've used something like this since the beginning
> of the TO
> Perl -> Go rewrite effort, but we still do have a decent
> amount of
> routes left that this could be useful for.
>
> Please let me know what you think. If we're generally +1 on
> this idea,
> I'll throw together a blueprint with more details, then maybe
> I can
> convince my boss to let me work on it ;)
>
> - Rawlin
>
>
>
>
>
>
>
Re: [EXTERNAL] TO API routing blacklist
Posted by "Gray, Jonathan" <Jo...@comcast.com>.
The available routes and what associated backend they go to are operator+ sensitive information. I'd also lean towards a stronger degree of safety and only make it read-only.
Jonathan G
On 11/1/19, 8:41 AM, "Hoppal, Michael" <Mi...@comcast.com> wrote:
I would be +1 on TO API routing blacklist.
I think it is a great idea to allow us to continue to move fast while minimizing risk.
I do agree we should expose the current state of the blacklist as an API endpoint as to easily get insight into the current configuration. We could even make the blacklist driven by the API instead of a JSON config file.
Michael
On 10/31/19, 4:18 PM, "Gray, Jonathan" <Jo...@comcast.com> wrote:
Not trying to sideswipe, but could we expose that as an endpoint with a Golang list as well to solve: https://protect2.fireeye.com/url?k=1978833f-459c8df4-1978a48b-000babff3540-c717b11942d8382e&u=https://github.com/apache/trafficcontrol/issues/2872
Jonathan G
On 10/31/19, 3:51 PM, "Rawlin Peters" <ra...@apache.org> wrote:
Hey all,
We've been picking up momentum in the TO Perl -> Go rewrite recently,
which has gotten me thinking of ways to reduce the risk of possible
regressions in the rewritten APIs. I'd like to propose that we
implement a sort of "routing blacklist" for the TO API routes.
At a high level, this blacklist would be a simple JSON config file like this:
{
"perlRoutes": [
{"method": "GET", "path": "/api/1.1/foos"},
{"method": "POST", "path": "/api/1.1/foos"}
],
"disabledRoutes": [
{"method": "GET", "path": "/api/1.1/foos"},
{"method": "POST", "path": "/api/1.1/foos"}
]
}
APIs in "perlRoutes" would be explicitly routed to the Perl, even if
the Go route exists and could handle the request. The primary use case
for this feature would be to deploy an upgraded version of TO-Go with
rewritten APIs without having TO-Go handle the requests yet.
Post-upgrade, you could remove a rewritten route from the blacklist on
a single TO instance, validate it for some period of time, then roll
out the rewritten API to other TO instances by removing the API from
their blacklists.
APIs in "disabledRoutes" would be explicitly disabled. The use cases
for this field would include:
- disabling endpoints that are part of an incomplete feature and don't
really make sense to use on their own yet
- disabling endpoints that have known, serious issues that should be
disabled immediately
This would make it easier to plug holes in TO without having to
rebuild and redeploy.
Ideally, this config file would be SIGHUP-able so that it can be
reconfigured without having to restart TO. Also, there should be a
hardcoded list of "Perl-routable" routes within TO-Go, so that you
can't just depend on Perl forever if you wanted. I would propose that
newly rewritten routes be added to this hardcoded list in TO-Go. Then
after being in one release as "Perl-routable", they would become
"non-Perl-routable" in the following release.
IMO we could've used something like this since the beginning of the TO
Perl -> Go rewrite effort, but we still do have a decent amount of
routes left that this could be useful for.
Please let me know what you think. If we're generally +1 on this idea,
I'll throw together a blueprint with more details, then maybe I can
convince my boss to let me work on it ;)
- Rawlin
Re: [EXTERNAL] TO API routing blacklist
Posted by "Hoppal, Michael" <Mi...@comcast.com>.
I would be +1 on TO API routing blacklist.
I think it is a great idea to allow us to continue to move fast while minimizing risk.
I do agree we should expose the current state of the blacklist as an API endpoint as to easily get insight into the current configuration. We could even make the blacklist driven by the API instead of a JSON config file.
Michael
On 10/31/19, 4:18 PM, "Gray, Jonathan" <Jo...@comcast.com> wrote:
Not trying to sideswipe, but could we expose that as an endpoint with a Golang list as well to solve: https://github.com/apache/trafficcontrol/issues/2872
Jonathan G
On 10/31/19, 3:51 PM, "Rawlin Peters" <ra...@apache.org> wrote:
Hey all,
We've been picking up momentum in the TO Perl -> Go rewrite recently,
which has gotten me thinking of ways to reduce the risk of possible
regressions in the rewritten APIs. I'd like to propose that we
implement a sort of "routing blacklist" for the TO API routes.
At a high level, this blacklist would be a simple JSON config file like this:
{
"perlRoutes": [
{"method": "GET", "path": "/api/1.1/foos"},
{"method": "POST", "path": "/api/1.1/foos"}
],
"disabledRoutes": [
{"method": "GET", "path": "/api/1.1/foos"},
{"method": "POST", "path": "/api/1.1/foos"}
]
}
APIs in "perlRoutes" would be explicitly routed to the Perl, even if
the Go route exists and could handle the request. The primary use case
for this feature would be to deploy an upgraded version of TO-Go with
rewritten APIs without having TO-Go handle the requests yet.
Post-upgrade, you could remove a rewritten route from the blacklist on
a single TO instance, validate it for some period of time, then roll
out the rewritten API to other TO instances by removing the API from
their blacklists.
APIs in "disabledRoutes" would be explicitly disabled. The use cases
for this field would include:
- disabling endpoints that are part of an incomplete feature and don't
really make sense to use on their own yet
- disabling endpoints that have known, serious issues that should be
disabled immediately
This would make it easier to plug holes in TO without having to
rebuild and redeploy.
Ideally, this config file would be SIGHUP-able so that it can be
reconfigured without having to restart TO. Also, there should be a
hardcoded list of "Perl-routable" routes within TO-Go, so that you
can't just depend on Perl forever if you wanted. I would propose that
newly rewritten routes be added to this hardcoded list in TO-Go. Then
after being in one release as "Perl-routable", they would become
"non-Perl-routable" in the following release.
IMO we could've used something like this since the beginning of the TO
Perl -> Go rewrite effort, but we still do have a decent amount of
routes left that this could be useful for.
Please let me know what you think. If we're generally +1 on this idea,
I'll throw together a blueprint with more details, then maybe I can
convince my boss to let me work on it ;)
- Rawlin