zalando / skipper Goto Github PK

it could be tested if /some/path/** works, and then it could be mentioned in the documentation.

#28 (comment)
#28 (comment)

currently the requestHeader and responseHeader filters use the stdlib add method, which in some cases may be what the user expects and in some cases may not.

• enable set besides add
• improve the naming

Github provides a good source of route definitions that can be easily reviewed and versioned

zalando/skipper/innkeeper/client.go:346

In case of a 500, the calling code will believe that the request was successful, and can't even make a log entry or anything.

For

src/github.com/zalando/skipper/cmd/eskip/media.go:137

Refactor the map keeping in mind @mo-gr's comment

To me, those two maps (commandToValidations and commands in eskip.go) smell a lot like some 
weird inside-out data structures (call it OO if you insist). I think, command should not be a type alias for 
a string anymore but instead be a proper struct containing a command-name (the current command), a 
validation function pointer and a commandFunc function pointer. This way, the weird map lookups 
would go away and we could do proper validations and command execution dispatched on the actual
command.

tests are randomly failing due to the non-sync initial propagation of the route settings. however, this is not an issue in normal operation, it causes annoyances in dev time and ci/cd.

Some filters need access to the backend url from the route to whitelist/blacklist certain features

Currently there is no way to do it.

The FilterContext should include a copy of that element from the route

#28 (comment)

Skipper needs some branding

Right now, as soon as a predicate or filter is constructed, there are some checks done.

Those checks can be done in a more general, declarative way. For example, for a status filter, we could have:

status(float)

Based on this, we know that status should accept one parameter, and that that parameter should be a float.

We should also expose counter metrics for, at least, the major errors

Pre-Route filters allows us to always go through a set of filters that are relevant enough to be applied to each and every route. Owners of the routes don't need to include them and they also can't opt-out.

Post-Route filters can be used to blacklist given headers / cookies, for ex. or override specific headers like 'X-Powered-By', for ex.

Implement the delete operation in eskip for Innkeeper

Either remove RouteInfo from everything but etcd, either implement route parsing errors for innkeeper

'routeInfos' here is visible in the docs, due to LoadAndParseAll is public, info doesn't have plural form. Please, give it a better name.

consider if this should be done by eskip, or just provide an awk example

• Return gzip response when Accept-Encoding: gzip, deflate is set by the browser

#28 (comment)

allow this:

chainedFilter(p1, p2): filter1("value", p1) -> filter2(1, p2, 2, 3);
noopFilter():;
route1: noopFilter() -> chainedFilter("param1", 1.5) -> otherFilter() -> ":8080"

involves:

• eskip syntax
• runtime route table construction
• etcd: (with prefix /filters on the same level as /routes)
• innkeeper
• eskip tool

Write now, we create it with the insecure flag. writeclient.go:32

If I have the following three routes:

a: Path("/foo/*_") -> "https://foo.org"
b: Path("/foo/bar") && Traffic(0.5, "bar", "foobar") -> "https://bar.org"
z: * -> "https://catch.all"

I would expect a request with the path /foo/bar to hit foo.org in half the time and bar.org the other half. However, all the requests, that don't hit bar.org end up being route to catch.all.

This is probably due to the way, the path-match tree is constructed, but it is very surprising and dangerous behaviour.

As of current version (08b8adf), skipper needs some linting love: it currently has 278 "problems", mostly style mistakes like proxy/proxy.go:228:6: func cloneUrl should be cloneURL.
I would go through all the source code and make it compliant. Please @lmineiro @aryszka have a look at the current output and share what you think.

as an extra, may check if a buffer pool pays back the cost of the additional complexity

To add Skipper to https://github.com/avelino/awesome-go, it needs a Gocover link. Can you add?

Would also identify 2-3 issues you really want help with that are good candidates for community to-do's, and add them in the README. I could do the add, if you share the issue #'s here.

should be similar to setting %{Host}i in apache

Often there is a flow id that helps gather events and log entries from multiple systems originated for the same customer request.

Such flow id needs to be generated at the earliest and sent down the request path so that the following systems can use it for those purposes.

I suggest we use a common HTTP header X-Flow-Id that is created for every request. Generation of this id should favor performance and relax on collision probability since the time window where this is relevant is limited

#28 (comment)

Pasting on behalf of @mo-gr :

The incoming req/resp and the outgoing req/resp in some filters can be confusing. I believe, the likelyhood of such issues can be greatly reduced by some type-trickery.
package main

import (
"fmt"
"net/http"
)

type IncomingRequest http.Request
type OutgoingRequest http.Request

func foo(r http.Request) int {
    return 1
}

func Filter(r IncomingRequest) int {
    return 2
}

func main() {
    fmt.Println(Filter(IncomingRequest{}))
    // fmt.Println(Filter(OutgoingRequest{})) // fails
    fmt.Println(foo(http.Request(IncomingRequest{})))
}

This would make it clear, which is which and you could write functions, that are only callable with inbound requests (or outbound responses or whatever)
I am aware that this is a rather huge refactoring. And I'm not 100% sure if the benefits are worth it
Most annoyingly, it would completely break the Filter/Predicate API
Maybe something to consider for a version 2?

This should work

PathRegexp(/[/]/)

but it fails in eskip

We need a global request metric and also a routing failure metric

Can we add some simple steps/details here? Then we can push as a "Help Wanted."

• It should be possible to turn on and off.
• It should be possible to specify Key, CA, Certificate.