Allow to cleanup resources for filters

[grassator]

Currently filters instantiated using a createFilter method can leak memory or other resources if they are requisitioned on creation and the corresponding route is consequently removed from a skipper instance.

The proposal is to add an additional interface filter.ManagedFilter that would have a single Close() method that is called whenever a route is removed from a skipper instance.

[LappleApple]

Hey @grassator @aryszka @Raffo @szuecs @mo-gr : Has this been done? If not, and you still need it, let's add a "Help Wanted" label.

[aryszka]

this is relevant. At least one service being implemented may need it in the near future. Adding the Help Wanted label.

[AlexanderYastrebov]

Currently filters instantiated using a createFilter method can leak memory or other resources if they are requisitioned on creation and the corresponding route is consequently removed from a skipper instance.

All routes (and hence all filters) are recreated on any route update because whole route table is re-instantiated, as can be demonstrated by:

diff --git a/routing/datasource.go b/routing/datasource.go
index 3eaa44c..c772e94 100644
--- a/routing/datasource.go
+++ b/routing/datasource.go
@@ -230,7 +230,7 @@ func createFilter(fr filters.Registry, def *eskip.Filter, cpm map[string]Predica

                return nil, fmt.Errorf("filter not found: '%s'", def.Name)
        }
-
+       fmt.Printf("create filter: %v %v\n", def.Name, def.Args)
        return spec.CreateFilter(def.Args)
 }

This means that all filters from previous version of routing table should be cleaned up. In order to perform safe cleanup there must be no in-flight requests that use route from previous version of route table. To achieve that route table (or particular route within it) has to track request start and completion. This in turn would probably require a move away from simple atomic.Value load/store https://github.com/zalando/skipper/blob/233e1d33f24e0f3ef062488fbb99bfb187c1cebf/routing/routing.go#L390-L393 https://github.com/zalando/skipper/blob/233e1d33f24e0f3ef062488fbb99bfb187c1cebf/routing/routing.go#L338-L344 to some critical section that would not only guard table access but also track acquisitions (e.g. with a sync.WaitGroup)

It is also interesting that unimplemented Close() filter method is mentioned in the docs: https://github.com/zalando/skipper/blame/master/docs/reference/development.md#L171-L174 https://github.com/zalando/skipper/blame/master/docs/tutorials/development.md#L38-L41

and some filters even define it https://github.com/zalando/skipper/blob/233e1d33f24e0f3ef062488fbb99bfb187c1cebf/filters/auth/webhook.go#L129-L132 https://github.com/zalando/skipper/blob/233e1d33f24e0f3ef062488fbb99bfb187c1cebf/filters/auth/tokeninfo.go#L361-L364 https://github.com/zalando/skipper/blob/233e1d33f24e0f3ef062488fbb99bfb187c1cebf/filters/auth/tokenintrospection.go#L478-L481 https://github.com/zalando/skipper/blob/233e1d33f24e0f3ef062488fbb99bfb187c1cebf/filters/auth/oidc.go#L813-L816