Skip to content
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

Merged
merged 2 commits into from
Oct 31, 2015

Conversation

archisgore
Copy link

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.

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.
archisgore pushed a commit to polyverse-security/vulcand that referenced this pull request Sep 24, 2015
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.
archisgore pushed a commit to polyverse-security/vulcand that referenced this pull request Oct 16, 2015
    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
@archisgore
Copy link
Author

Gentle reminder - can you please take a look at this? This helps Router be an interface instead of a hardcoded struct.

@ghost
Copy link

ghost commented Oct 23, 2015

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

  1. Convert a multi-part as base64 and replace it into a JSON payload where I can replace the string inside. (Input transform middleware with a template per route)
  2. Combine several routes (up to 10), (like https://github.com/lepidosteus/golang-http-batch/blob/master/exemple/exemple.go) and nest my output into one JSON response.

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

archisgore pushed a commit that referenced this pull request Oct 31, 2015
Hide NotFound struct member from Mux, and expose as an interface.
@archisgore archisgore merged commit 0c31bc1 into vulcand:master Oct 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant