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
<[email protected]> 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" <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
> 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 <
> > [email protected]>
> > > > 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" <[email protected]> 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
> > > > >
> > > > >
> > > >
> >
>
>
