[experimental] Move admission webhook into skipper for better validation

Signed-off-by: Mustafa Abdelrahman <[email protected]>

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 {
@@ -294,7 +295,19 @@ type Config struct {
 	OpenPolicyAgentMaxRequestBodySize    int64         `yaml:"open-policy-agent-max-request-body-size"`
 	OpenPolicyAgentMaxMemoryBodyParsing  int64         `yaml:"open-policy-agent-max-memory-body-parsing"`
 
-	PassiveHealthCheck mapFlags `yaml:"passive-health-check"`
+	PassiveHealthCheck             mapFlags      `yaml:"passive-health-check"`
+	EnableOpenPolicyAgent          bool          `yaml:"enable-open-policy-agent"`
+	OpenPolicyAgentConfigTemplate  string        `yaml:"open-policy-agent-config-template"`
+	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 (
@@ -595,6 +608,13 @@ func NewConfig() *Config {
 	flag.Var(&cfg.PassiveHealthCheck, "passive-health-check", "sets the parameters for passive health check feature")
 
 	cfg.Flags = flag
+	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
 }
 
@@ -944,6 +964,12 @@ func (c *Config) ToOptions() skipper.Options {
 		OpenPolicyAgentMaxMemoryBodyParsing:  c.OpenPolicyAgentMaxMemoryBodyParsing,
 
 		PassiveHealthCheck: c.PassiveHealthCheck.values,
+		// 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

@@ -164,6 +164,8 @@ func defaultConfig(with func(*Config)) *Config {
 		OpenPolicyAgentMaxRequestBodySize:       openpolicyagent.DefaultMaxRequestBodySize,
 		OpenPolicyAgentMaxMemoryBodyParsing:     openpolicyagent.DefaultMaxMemoryBodyParsing,
 		OpenPolicyAgentRequestBodyBufferSize:    openpolicyagent.DefaultRequestBodyBufferSize,
+		ValidationWebhookAddr:                   ":9443",
+		ValidationWebhookLogLevel:               "info",
 	}
 	with(cfg)
 	return cfg

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

@@ -5,9 +5,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
@@ -20,14 +23,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 {
@@ -42,12 +50,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 errors.Join(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)
@@ -27,9 +27,6 @@ skipper:
 eskip:
 	GOOS=$(GOOS) GOARCH=$(GOARCH) $(GOARM) CGO_ENABLED=$(CGO_ENABLED) go build $(BUILD_FLAGS) -o eskip ../cmd/eskip/*.go
 
-webhook:
-	GOOS=$(GOOS) GOARCH=$(GOARCH) $(GOARM) CGO_ENABLED=$(CGO_ENABLED) go build $(BUILD_FLAGS) -o webhook ../cmd/webhook/*.go
-
 routesrv:
 	GOOS=$(GOOS) GOARCH=$(GOARCH) $(GOARM) CGO_ENABLED=$(CGO_ENABLED) go build $(BUILD_FLAGS) -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 (
@@ -947,6 +948,21 @@ type Options struct {
 	OpenPolicyAgentMaxMemoryBodyParsing  int64
 
 	PassiveHealthCheck map[string]string
+	// 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 {
@@ -2058,6 +2074,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 4: 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)