-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hide NotFound struct member from Mux, and expose as an interface. #10
Conversation
We want to reuse the routing library for other applications, and also to allow unit testing of vulcan by plugging in mock routing libraries, or experimental/test routing libraries. By having NotFound as a public member, it prevents us from plugging in a replacement interface. By making all implementation private, and exposed through getters/setters, it allows us to make better effective use of route. Since vulcan uses godeps to base library versions, I made the potentially breaking change of renaming NotFound to notFound. This could potentially break any consumers who rely on it being public, but I think this chance is for the better. Insofar as vulcan itself is concerned, I'll submit a change that rebases against this route implementation and uses SetNotFound methods instead of assigning directly to NotFound.
I filed an ISSUE earlier to discuss this here: vulcand#218 The crux of this change is: 1. To make Router an interface behind which implementations can be changed out. 2. To allow the Registry to specify an implementation. 3. To fall back to the default implementation (route.Mux) when one is not provided by the registry. This change also depends on this pending PR in the mailgun/route library vulcand/route#10 WHY? As we try to add new features to mailgun/route, we continue to have to work off custom branches and re-vendor the files, and rebuild vulcand for each experiment. For example we have a forked route library which contains massive amounts of logging. Having re-vendored files is always confusing because we don't know what branch/fork it came from. We'd much rather have a way to host 10 forks and simply plug in an implementation at runtime based on command-line flags. This allows us to rapidly innovate, and I'm sure gives vulcand users the ability to use richer or simpler languages. For instance, one experiment I want to run is use of Javascript-based expressions which would allow the use of VERY rich matching rules at the cost of VERY high compute load per rule-match. It's just an idea, but trying it out can be made easier by making it pluggable. As it happens this change doesn't affect how Vulcan works today, and the defaults do not change.
I filed an ISSUE earlier to discuss this here: vulcand#218 The crux of this change is: 1. To make Router an interface behind which implementations can be changed out. 2. To allow the Registry to specify an implementation. 3. To fall back to the default implementation (route.Mux) when one is not provided by the registry. This change also depends on this pending PR in the mailgun/route library vulcand/route#10 WHY? As we try to add new features to mailgun/route, we continue to have to work off custom branches and re-vendor the files, and rebuild vulcand for each experiment. For example we have a forked route library which contains massive amounts of logging. Having re-vendored files is always confusing because we don't know what branch/fork it came from. We'd much rather have a way to host 10 forks and simply plug in an implementation at runtime based on command-line flags. This allows us to rapidly innovate, and I'm sure gives vulcand users the ability to use richer or simpler languages. For instance, one experiment I want to run is use of Javascript-based expressions which would allow the use of VERY rich matching rules at the cost of VERY high compute load per rule-match. It's just an idea, but trying it out can be made easier by making it pluggable. As it happens this change doesn't affect how Vulcan works today, and the defaults do not change. Some example routing rules are: r.Referrer.indexOf("foo.com") > 0 //When referrer is foo.com r.Header.Get("Cookie")["foo"] == "bar" //Check value of cookie without using a complex regex TESTING: There are sufficient UTs here, and in addition we have written sufficient UTs in a couple of routing libraries we wrote as an experiment to see how far we could go. Polyverse Corporation (the company I work for) has been using the forked copy with these changes for over three weeks at this point. We're running a pluggable library in production across a large cluster reliably. HOW TO: After this change, this is how we inject a custom router into our own installation: func GetRegistry(selfaddress string) (*plugin.Registry, error) { r := plugin.NewRegistry() specs := []*plugin.MiddlewareSpec{ connlimit.GetSpec(), ratelimit.GetSpec(), rewrite.GetSpec(), cbreaker.GetSpec(), trace.GetSpec(), ttlresetmiddleware.GetSpec(), unroutedrequesthandler.GetSpec(), polyverseerrormiddleware.GetSpec(), connectionmanagementmiddleware.GetSpec(), } r.AddNotFoundMiddleware(notFoundMiddleware) //Use combination-router to enable javascript evaluation as a fallback mechanism //when mailgun/route is insufficient. routers := combineroute.Routers{ combineroute.RouterEntry{ Name: "vulcanrouter", Router: route.NewMux(), }, combineroute.RouterEntry{ Name: "javascriptrouter", Router: jsroute.NewMux(), }, } commonrouter := combineroute.NewMux(routers) r.SetRouter(commonrouter) for _, spec := range specs { if err := r.AddSpec(spec); err != nil { return nil, err } } return r, nil } READY TO USE ROUTERS: Here are a couple of libraries that demonstrate the possibilities of allowing different language evaluators to be used for routing rules. https://github.com/polyverse-security/js-route https://github.com/polyverse-security/combine-route
Gentle reminder - can you please take a look at this? This helps Router be an interface instead of a hardcoded struct. |
+1 that's really cool what you have done @archisgore :-) Mailgun team too :-) I mean it has a huge potential... @archisgore, I am working in Computer Vision and I would like to combine several routes but also do some processing on the input:
I am sure that storing the JSON payload into ECTD for a set of routes would be the ideal solution to make some test without reloading vulcand. Just miss a good UI to manage it but I failed to used Vulcan-Salute, also it does not include middlewares exploration. Is your fork close to do that ? Do you have any insights that could help me to make it cool and sweet ? :-) Luc |
Hide NotFound struct member from Mux, and expose as an interface.
We want to reuse the routing library for other applications, and also
to allow unit testing of vulcan by plugging in mock routing libraries,
or experimental/test routing libraries.
By having NotFound as a public member, it prevents us from plugging in
a replacement interface. By making all implementation private, and exposed
through getters/setters, it allows us to make better effective use of route.
Since vulcan uses godeps to base library versions, I made the potentially
breaking change of renaming NotFound to notFound. This could
potentially break any consumers who rely on it being public, but I think
this chance is for the better. Insofar as vulcan itself is concerned, I'll submit
a change that rebases against this route implementation and uses SetNotFound
methods instead of assigning directly to NotFound.