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 > > > > > > > > > > > >
