[experimental] Move admission webhook into skipper for better validation

Signed-off-by: Mustafa Abdelrahman <mustafa.abdelrahman@zalando.de>

Makefile

@@ -31,10 +31,6 @@ skipper: $(SOURCES) ## build skipper binary
 eskip: $(SOURCES) ## build eskip binary
 	go build -ldflags "-X main.version=$(VERSION) -X main.commit=$(COMMIT_HASH)" -o bin/eskip ./cmd/eskip
 
-.PHONY: webhook
-webhook: $(SOURCES) ## build webhook binary
-	go build -ldflags "-X main.version=$(VERSION) -X main.commit=$(COMMIT_HASH)" -o bin/webhook ./cmd/webhook
-
 .PHONY: routesrv
 routesrv: $(SOURCES) ## build routesrv binary
 	go build -ldflags "-X main.version=$(VERSION) -X main.commit=$(COMMIT_HASH)" -o bin/routesrv ./cmd/routesrv
@@ -46,7 +42,7 @@ ifeq (LIMIT_FDS, 256)
 endif
 
 .PHONY: build
-build: $(SOURCES) lib skipper eskip webhook routesrv ## build library and all binaries
+build: $(SOURCES) lib skipper eskip routesrv ## build library and all binaries
 
 build.linux.static: ## build static linux binary for amd64
 	GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build -o bin/skipper -ldflags "-extldflags=-static -X main.version=$(VERSION) -X main.commit=$(COMMIT_HASH)" ./cmd/skipper

config/config.go

@@ -23,6 +23,7 @@ import (
 	"github.com/zalando/skipper/net"
 	"github.com/zalando/skipper/proxy"
 	"github.com/zalando/skipper/swarm"
+	"github.com/zalando/skipper/webhook"
 )
 
 type Config struct {
@@ -283,6 +284,13 @@ type Config struct {
 	OpenPolicyAgentEnvoyMetadata   string        `yaml:"open-policy-agent-envoy-metadata"`
 	OpenPolicyAgentCleanerInterval time.Duration `yaml:"open-policy-agent-cleaner-interval"`
 	OpenPolicyAgentStartupTimeout  time.Duration `yaml:"open-policy-agent-startup-timeout"`
+	// admission webhook
+	// validation addmission webhook
+	EnableValidationWebhook      bool   `yaml:"enable-validation-webhook"`
+	ValidationWebhookTLSCertFile string `yaml:"validation-webhook-tls-cert-file"`
+	ValidationWebhookTLSKeyFile  string `yaml:"validation-webhook-tls-key-file"`
+	ValidationWebhookAddr        string `yaml:"validation-webhook-address"`
+	ValidationWebhookLogLevel    string `yaml:"validation-webhook-log-level"`
 }
 
 const (
@@ -567,6 +575,12 @@ func NewConfig() *Config {
 	flag.Var(cfg.LuaModules, "lua-modules", "comma separated list of lua filter modules. Use <module>.<symbol> to selectively enable module symbols, for example: package,base._G,base.print,json")
 	flag.Var(cfg.LuaSources, "lua-sources", `comma separated list of lua input types for the lua() filter. Valid sources "", "file", "inline", "file,inline" and "none". Use "file" to only allow lua file references in lua filter. Default "" is the same as "file","inline". Use "none" to disable lua filters.`)
 
+	flag.BoolVar(&cfg.EnableValidationWebhook, "enable-validation-webhook", false, "enables the validation admission webhook for RouteGroup CRD, *IMPORTANT* This mode runs only the validation webhook server and does not start the proxy")
+	flag.StringVar(&cfg.ValidationWebhookTLSCertFile, "validation-webhook-tls-cert-file", os.Getenv("CERT_FILE"), "File containing the certificate for HTTPS")
+	flag.StringVar(&cfg.ValidationWebhookTLSKeyFile, "validation-webhook-tls-key-file", os.Getenv("KEY_FILE"), "File containing the private key for HTTPS")
+	flag.StringVar(&cfg.ValidationWebhookAddr, "validation-webhook-address", webhook.DefaultHTTPSAddress, "The address to listen on")
+	flag.StringVar(&cfg.ValidationWebhookLogLevel, "validation-webhook-log-level", webhook.DefaultLogLevel, "Log level for validation webhook server")
+
 	cfg.flags = flag
 	return cfg
 }
@@ -906,6 +920,12 @@ func (c *Config) ToOptions() skipper.Options {
 		OpenPolicyAgentEnvoyMetadata:   c.OpenPolicyAgentEnvoyMetadata,
 		OpenPolicyAgentCleanerInterval: c.OpenPolicyAgentCleanerInterval,
 		OpenPolicyAgentStartupTimeout:  c.OpenPolicyAgentStartupTimeout,
+		// Admission Webhook:
+		EnableValidationWebhook:      c.EnableValidationWebhook,
+		ValidationWebhookTLSCertFile: c.ValidationWebhookTLSCertFile,
+		ValidationWebhookTLSKeyFile:  c.ValidationWebhookTLSKeyFile,
+		ValidationWebhookAddr:        c.ValidationWebhookAddr,
+		ValidationWebhookLogLevel:    c.ValidationWebhookLogLevel,
 	}
 	for _, rcci := range c.CloneRoute {
 		eskipClone := eskip.NewClone(rcci.Reg, rcci.Repl)

config/config_test.go

@@ -162,6 +162,8 @@ func defaultConfig() *Config {
 		LuaSources:                              commaListFlag(),
 		OpenPolicyAgentCleanerInterval:          10 * time.Second,
 		OpenPolicyAgentStartupTimeout:           30 * time.Second,
+		ValidationWebhookAddr:                   ":9443",
+		ValidationWebhookLogLevel:               "info",
 	}
 }
 

dataclients/kubernetes/clusterclient.go

@@ -173,8 +173,8 @@ func newClusterClient(o Options, apiURL, ingCls, rgCls string, quit <-chan struc
 		httpClient:                   httpClient,
 		apiURL:                       apiURL,
 		certificateRegistry:          o.CertificateRegistry,
-		routeGroupValidator:          &definitions.RouteGroupValidator{},
-		ingressValidator:             &definitions.IngressV1Validator{},
+		ingressValidator:             &definitions.IngressV1Validator{FiltersRegistry: o.FiltersRegistry},
+		routeGroupValidator:          &definitions.RouteGroupValidator{FiltersRegistry: o.FiltersRegistry},
 		enableEndpointSlices:         o.KubernetesEnableEndpointslices,
 	}
 

dataclients/kubernetes/definitions/ingressvalidator.go

@@ -4,9 +4,12 @@ import (
 	"fmt"
 
 	"github.com/zalando/skipper/eskip"
+	"github.com/zalando/skipper/filters"
 )
 
-type IngressV1Validator struct{}
+type IngressV1Validator struct {
+	FiltersRegistry filters.Registry
+}
 
 func (igv *IngressV1Validator) Validate(item *IngressV1Item) error {
 	var errs []error
@@ -19,14 +22,19 @@ func (igv *IngressV1Validator) Validate(item *IngressV1Item) error {
 }
 
 func (igv *IngressV1Validator) validateFilterAnnotation(annotations map[string]string) error {
+	var errs []error
 	if filters, ok := annotations[IngressFilterAnnotation]; ok {
-		_, err := eskip.ParseFilters(filters)
+		parsedFilters, err := eskip.ParseFilters(filters)
 		if err != nil {
-			err = fmt.Errorf("invalid \"%s\" annotation: %w", IngressFilterAnnotation, err)
+			errs = append(errs, fmt.Errorf("invalid \"%s\" annotation: %w", IngressFilterAnnotation, err))
+		}
+
+		if igv.FiltersRegistry != nil {
+			errs = append(errs, igv.validateFiltersNames(parsedFilters))
 		}
-		return err
 	}
-	return nil
+
+	return errorsJoin(errs...)
 }
 
 func (igv *IngressV1Validator) validatePredicateAnnotation(annotations map[string]string) error {
@@ -41,12 +49,28 @@ func (igv *IngressV1Validator) validatePredicateAnnotation(annotations map[strin
 }
 
 func (igv *IngressV1Validator) validateRoutesAnnotation(annotations map[string]string) error {
+	var errs []error
 	if routes, ok := annotations[IngressRoutesAnnotation]; ok {
-		_, err := eskip.Parse(routes)
+		parsedRoutes, err := eskip.Parse(routes)
 		if err != nil {
-			err = fmt.Errorf("invalid \"%s\" annotation: %w", IngressRoutesAnnotation, err)
+			errs = append(errs, fmt.Errorf("invalid \"%s\" annotation: %w", IngressRoutesAnnotation, err))
+		}
+
+		if igv.FiltersRegistry != nil {
+			for _, r := range parsedRoutes {
+				errs = append(errs, igv.validateFiltersNames(r.Filters))
+			}
+		}
+	}
+
+	return errorsJoin(errs...)
+}
+
+func (igv *IngressV1Validator) validateFiltersNames(filters []*eskip.Filter) error {
+	for _, f := range filters {
+		if _, ok := igv.FiltersRegistry[f.Name]; !ok {
+			return fmt.Errorf("filter \"%s\" is not supported", f.Name)
 		}
-		return err
 	}
 	return nil
 }

dataclients/kubernetes/definitions/routegroupvalidator.go

@@ -6,9 +6,12 @@ import (
 	"net/url"
 
 	"github.com/zalando/skipper/eskip"
+	"github.com/zalando/skipper/filters"
 )
 
-type RouteGroupValidator struct{}
+type RouteGroupValidator struct {
+	FiltersRegistry filters.Registry
+}
 
 var (
 	errSingleFilterExpected    = errors.New("single filter expected")
@@ -72,13 +75,24 @@ func (rgv *RouteGroupValidator) validateFilters(item *RouteGroupItem) error {
 				errs = append(errs, err)
 			} else if len(filters) != 1 {
 				errs = append(errs, fmt.Errorf("%w at %q", errSingleFilterExpected, f))
+			} else if rgv.FiltersRegistry != nil {
+				errs = append(errs, rgv.validateFiltersNames(filters))
 			}
 		}
 	}
 
 	return errorsJoin(errs...)
 }
 
+func (rgv *RouteGroupValidator) validateFiltersNames(filters []*eskip.Filter) error {
+	for _, f := range filters {
+		if _, ok := rgv.FiltersRegistry[f.Name]; !ok {
+			return fmt.Errorf("filter \"%s\" is not supported", f.Name)
+		}
+	}
+	return nil
+}
+
 func (rgv *RouteGroupValidator) validatePredicates(item *RouteGroupItem) error {
 	var errs []error
 	for _, r := range item.Spec.Routes {

dataclients/kubernetes/kube.go

@@ -240,6 +240,9 @@ type Options struct {
 	// DefaultLoadBalancerAlgorithm sets the default algorithm to be used for load balancing between backend endpoints,
 	// available options: roundRobin, consistentHash, random, powerOfRandomNChoices
 	DefaultLoadBalancerAlgorithm string
+
+	// FiltersRegistry is used to validate filters names in RouteGroups/Ingresses
+	FiltersRegistry filters.Registry
 }
 
 // Client is a Skipper DataClient implementation used to create routes based on Kubernetes Ingress settings.

packaging/Makefile

@@ -2,7 +2,7 @@
 
 VERSION            ?= $(shell git rev-parse HEAD)
 REGISTRY           ?= registry-write.opensource.zalan.do/teapot
-BINARIES           ?= skipper webhook eskip routesrv
+BINARIES           ?= skipper eskip routesrv
 IMAGE              ?= $(REGISTRY)/skipper:$(VERSION)
 ARM64_IMAGE        ?= $(REGISTRY)/skipper-arm64:$(VERSION)
 ARM_IMAGE          ?= $(REGISTRY)/skipper-armv7:$(VERSION)
@@ -25,9 +25,6 @@ skipper:
 eskip:
 	GOOS=$(GOOS) GOARCH=$(GOARCH) $(GOARM) CGO_ENABLED=$(CGO_ENABLED) go build -o eskip ../cmd/eskip/*.go
 
-webhook:
-	GOOS=$(GOOS) GOARCH=$(GOARCH) $(GOARM) CGO_ENABLED=$(CGO_ENABLED) go build -o webhook ../cmd/webhook/*.go
-
 routesrv:
 	GOOS=$(GOOS) GOARCH=$(GOARCH) $(GOARM) CGO_ENABLED=$(CGO_ENABLED) go build -o routesrv ../cmd/routesrv/*.go
 

skipper.go

@@ -69,6 +69,7 @@ import (
 	"github.com/zalando/skipper/secrets/certregistry"
 	"github.com/zalando/skipper/swarm"
 	"github.com/zalando/skipper/tracing"
+	"github.com/zalando/skipper/webhook"
 )
 
 const (
@@ -917,6 +918,22 @@ type Options struct {
 	OpenPolicyAgentEnvoyMetadata   string
 	OpenPolicyAgentCleanerInterval time.Duration
 	OpenPolicyAgentStartupTimeout  time.Duration
+
+	// EnableValidationWebhook runs skipper in admission webhook mode
+	//  *IMPORTANT* This mode runs only the validation webhook server and does not start the proxy
+	EnableValidationWebhook bool
+
+	// ValidationWebhookTLSCertFile is the path to the certificate file for the admission webhook server
+	ValidationWebhookTLSCertFile string
+
+	// ValidationWebhookTLSKeyFile is the path to the private key file for the admission webhook server
+	ValidationWebhookTLSKeyFile string
+
+	// ValidationWebhookAddr is the address to listen on for the admission webhook server
+	ValidationWebhookAddr string
+
+	// ValidationWebhookLogLevel is the log level for the admission webhook server
+	ValidationWebhookLogLevel string
 }
 
 func (o *Options) KubernetesDataClientOptions() kubernetes.Options {
@@ -1959,6 +1976,16 @@ func run(o Options, sig chan os.Signal, idleConnsCH chan struct{}) error {
 	routing := routing.New(ro)
 	defer routing.Close()
 
+	if o.EnableValidationWebhook {
+		webhook.Run(
+			o.ValidationWebhookLogLevel,
+			o.ValidationWebhookAddr,
+			o.ValidationWebhookTLSCertFile,
+			o.ValidationWebhookTLSKeyFile,
+			o.filterRegistry(),
+		)
+	}
+
 	proxyFlags := proxy.Flags(o.ProxyOptions) | o.ProxyFlags
 	proxyParams := proxy.Params{
 		Routing:                    routing,

cmd/webhook/admission/admission_test.go → webhook/admission/admission_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/zalando/skipper/dataclients/kubernetes/definitions"
+	"github.com/zalando/skipper/filters/builtin"
 )
 
 const (
@@ -87,9 +88,10 @@ func TestUnsupportedContentType(t *testing.T) {
 
 func TestRouteGroupAdmitter(t *testing.T) {
 	for _, tc := range []struct {
-		name      string
-		inputFile string
-		message   string
+		name               string
+		inputFile          string
+		message            string
+		withFilterRegistry bool
 	}{
 		{
 			name:      "allowed",
@@ -104,6 +106,12 @@ func TestRouteGroupAdmitter(t *testing.T) {
 			name:      "valid eskip filters",
 			inputFile: "rg-with-valid-eskip-filters.json",
 		},
+		{
+			name:               "valid eskip filters but not supported",
+			inputFile:          "rg-with-valid-eskip-filters-but-not-supported.json",
+			message:            `filter \"test\" is not supported`,
+			withFilterRegistry: true,
+		},
 		{
 			name:      "invalid eskip filters",
 			inputFile: "rg-with-invalid-eskip-filters.json",
@@ -157,7 +165,15 @@ func TestRouteGroupAdmitter(t *testing.T) {
 			req.Header.Set("Content-Type", "application/json")
 
 			w := httptest.NewRecorder()
-			rgAdm := &RouteGroupAdmitter{RouteGroupValidator: &definitions.RouteGroupValidator{}}
+
+			var rgValidator *definitions.RouteGroupValidator
+			if tc.withFilterRegistry {
+				rgValidator = &definitions.RouteGroupValidator{FiltersRegistry: builtin.MakeRegistry()}
+			} else {
+				rgValidator = &definitions.RouteGroupValidator{}
+			}
+
+			rgAdm := &RouteGroupAdmitter{RouteGroupValidator: rgValidator}
 
 			h := Handler(rgAdm)
 			h(w, req)
@@ -175,9 +191,10 @@ func TestRouteGroupAdmitter(t *testing.T) {
 
 func TestIngressAdmitter(t *testing.T) {
 	for _, tc := range []struct {
-		name      string
-		inputFile string
-		message   string
+		name               string
+		inputFile          string
+		message            string
+		withFilterRegistry bool
 	}{
 		{
 			name:      "allowed without annotations",
@@ -192,6 +209,16 @@ func TestIngressAdmitter(t *testing.T) {
 			inputFile: "invalid-filters.json",
 			message:   `invalid \"zalando.org/skipper-filter\" annotation: parse failed after token this, position 9: syntax error`,
 		},
+		{
+			name:               "Filter not found in filter registry",
+			inputFile:          "invalid-filter-name.json",
+			message:            `filter \"play\" is not supported`,
+			withFilterRegistry: true,
+		},
+		{
+			name:      "Filter not found in filter registry but valid eskip filters",
+			inputFile: "invalid-filter-name.json",
+		},
 		{
 			name:      "invalid eskip predicates",
 			inputFile: "invalid-predicates.json",
@@ -221,7 +248,15 @@ func TestIngressAdmitter(t *testing.T) {
 			req.Header.Set("Content-Type", "application/json")
 
 			w := httptest.NewRecorder()
-			ingressAdm := &IngressAdmitter{IngressValidator: &definitions.IngressV1Validator{}}
+
+			var ingressValidator *definitions.IngressV1Validator
+			if tc.withFilterRegistry {
+				ingressValidator = &definitions.IngressV1Validator{FiltersRegistry: builtin.MakeRegistry()}
+			} else {
+				ingressValidator = &definitions.IngressV1Validator{}
+			}
+
+			ingressAdm := &IngressAdmitter{IngressValidator: ingressValidator}
 
 			h := Handler(ingressAdm)
 			h(w, req)