refactor: Introduce a validator to validate envoy configs

changelog/v1.17.0-beta27/refactor-transformation-validation.yaml

@@ -0,0 +1,5 @@
+changelog:
+- type: NON_USER_FACING
+  description: >
+    Refactor `bootstrap.ValidateBootstrap()` by moving the `DisableTransformationValidation` check into the transformation plugin. This way the `bootstrap.ValidateBootstrap()` can be used in other areas independent of this setting.
+    Introduce a new Validator that validates an envoy config by running it by envoy in validate mode.

projects/gloo/pkg/bootstrap/bootstrap_validation.go

@@ -16,7 +16,6 @@ import (
 	"github.com/golang/protobuf/proto"
 	"github.com/golang/protobuf/ptypes/any"
 	"github.com/rotisserie/eris"
-	v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1"
 	"github.com/solo-io/gloo/projects/gloo/pkg/utils"
 	"github.com/solo-io/go-utils/contextutils"
 )
@@ -33,15 +32,9 @@ func getEnvoyPath() string {
 
 func ValidateBootstrap(
 	ctx context.Context,
-	settings *v1.Settings,
 	filterName string,
 	msg proto.Message,
 ) error {
-	// If the user has disabled transformation validation, then always return nil
-	if settings.GetGateway().GetValidation().GetDisableTransformationValidation().GetValue() {
-		return nil
-	}
-
 	bootstrapYaml, err := buildPerFilterBootstrapYaml(filterName, msg)
 	if err != nil {
 		return err

projects/gloo/pkg/plugins/transformation/plugin.go

@@ -7,9 +7,8 @@ import (
 
 	"github.com/golang/protobuf/proto"
 	"github.com/rotisserie/eris"
-	"github.com/solo-io/go-utils/contextutils"
+	"github.com/solo-io/gloo/projects/gloo/pkg/plugins/utils/validator"
 	"google.golang.org/protobuf/types/known/wrapperspb"
-	"k8s.io/utils/lru"
 
 	envoy_config_route_v3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
 	envoy_type_matcher_v3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3"
@@ -22,7 +21,6 @@ import (
 	v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1"
 	"github.com/solo-io/gloo/projects/gloo/pkg/api/v1/core/matchers"
 	"github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/transformation"
-	"github.com/solo-io/gloo/projects/gloo/pkg/bootstrap"
 	"github.com/solo-io/gloo/projects/gloo/pkg/plugins"
 	"github.com/solo-io/gloo/projects/gloo/pkg/plugins/pluginutils"
 )
@@ -49,8 +47,6 @@ var (
 	UnknownTransformationType = func(transformation interface{}) error {
 		return fmt.Errorf("unknown transformation type %T", transformation)
 	}
-	mCacheHits   = utils.MakeSumCounter("gloo.solo.io/transformation_validation_cache_hits", "The number of cache hits while validating transformation config")
-	mCacheMisses = utils.MakeSumCounter("gloo.solo.io/transformation_validation_cache_misses", "The number of cache misses while validating transformation config")
 )
 
 type TranslateTransformationFn func(*transformation.Transformation, *wrapperspb.BoolValue, *wrapperspb.BoolValue) (*envoytransformation.Transformation, error)
@@ -66,15 +62,17 @@ type Plugin struct {
 	TranslateTransformation    TranslateTransformationFn
 	settings                   *v1.Settings
 	logRequestResponseInfo     bool
-	// validationLruCache is a map of: (transformation hash) -> error state
-	// this is usually a typed error but may be an untyped nil interface
-	validationLruCache *lru.Cache
-	escapeCharacters   *wrapperspb.BoolValue
+	escapeCharacters           *wrapperspb.BoolValue
+	validator                  validator.Validator
 }
 
 func NewPlugin() *Plugin {
+	mCacheHits := utils.MakeSumCounter("gloo.solo.io/transformation_validation_cache_hits", "The number of cache hits while validating transformation config")
+	mCacheMisses := utils.MakeSumCounter("gloo.solo.io/transformation_validation_cache_misses", "The number of cache misses while validating transformation config")
+
 	return &Plugin{
-		validationLruCache: lru.New(1024),
+		validator: validator.New(ExtensionName, FilterName,
+			validator.WithCounters(mCacheHits, mCacheMisses)),
 	}
 }
 
@@ -536,36 +534,12 @@ func (p *Plugin) validateTransformation(
 	ctx context.Context,
 	transformations *envoytransformation.RouteTransformations,
 ) error {
-
-	transformHash, err := transformations.Hash(nil)
-	if err != nil {
-		contextutils.LoggerFrom(ctx).DPanicf("error hashing transformation, should never happen: %v", err)
-		return err
-	}
-
-	// This transformation has already been validated, return the result
-	if err, ok := p.validationLruCache.Get(transformHash); ok {
-		utils.MeasureOne(
-			ctx,
-			mCacheHits,
-		)
-		// Error may be nil here since it's just the cached result
-		// so return it as a nil err after cast worst case.
-		errCasted, _ := err.(error)
-		return errCasted
-	} else {
-		utils.MeasureOne(
-			ctx,
-			mCacheMisses,
-		)
+	// If the user has disabled transformation validation, then always return nil
+	if p.settings.GetGateway().GetValidation().GetDisableTransformationValidation().GetValue() {
+		return nil
 	}
 
-	err = bootstrap.ValidateBootstrap(ctx, p.settings, FilterName, transformations)
-	p.validationLruCache.Add(transformHash, err)
-	if err != nil {
-		return err
-	}
-	return nil
+	return p.validator.ValidateConfig(ctx, transformations)
 }
 
 func (p *Plugin) getTransformations(

projects/gloo/pkg/plugins/transformation/plugin_test.go

@@ -1,4 +1,4 @@
-package transformation_test
+package transformation
 
 import (
 	"context"
@@ -17,7 +17,6 @@ import (
 	"github.com/solo-io/gloo/projects/gloo/pkg/api/v1/core/matchers"
 	"github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/transformation"
 	"github.com/solo-io/gloo/projects/gloo/pkg/plugins"
-	. "github.com/solo-io/gloo/projects/gloo/pkg/plugins/transformation"
 	"github.com/solo-io/gloo/projects/gloo/pkg/utils"
 	skMatchers "github.com/solo-io/solo-kit/test/matchers"
 )
@@ -1082,4 +1081,53 @@ var _ = Describe("Plugin", func() {
 		})
 	})
 
+	Context("cache validation", func() {
+		var p *Plugin
+		var processRoute func()
+		var processAnotherRoute func()
+
+		BeforeEach(func() {
+			p = NewPlugin()
+			p.Init(plugins.InitParams{Ctx: ctx, Settings: &v1.Settings{Gloo: &v1.GlooOptions{RemoveUnusedFilters: &wrapperspb.BoolValue{Value: false}}}})
+
+			processRouteWithValue := func(value bool) {
+				err := p.ProcessRoute(plugins.RouteParams{
+					VirtualHostParams: plugins.VirtualHostParams{
+						Params: plugins.Params{
+							Ctx: ctx,
+						},
+					},
+				}, &v1.Route{
+					Options: &v1.RouteOptions{
+						Transformations: &transformation.Transformations{
+							ClearRouteCache: value,
+						},
+					},
+				}, &envoy_config_route_v3.Route{})
+				Expect(err).ToNot(HaveOccurred())
+			}
+
+			processRoute = func() {
+				processRouteWithValue(true)
+			}
+			processAnotherRoute = func() {
+				processRouteWithValue(false)
+			}
+		})
+
+		It("reuses the cache", func() {
+			processRoute()
+			Expect(p.validator.CacheLength()).To(Equal(1))
+
+			// When re-initializing the plugin, the cache is not cleared
+			p.Init(plugins.InitParams{Ctx: ctx, Settings: &v1.Settings{Gloo: &v1.GlooOptions{RemoveUnusedFilters: &wrapperspb.BoolValue{Value: false}}}})
+			Expect(p.validator.CacheLength()).To(Equal(1))
+
+			// The cache is still not cleared
+			p.Init(plugins.InitParams{Ctx: ctx, Settings: &v1.Settings{Gloo: &v1.GlooOptions{RemoveUnusedFilters: &wrapperspb.BoolValue{Value: false}}}})
+			processAnotherRoute()
+			Expect(p.validator.CacheLength()).To(Equal(2))
+		})
+
+	})
 })

projects/gloo/pkg/plugins/transformation/transformation_suite_test.go

@@ -1,4 +1,4 @@
-package transformation_test
+package transformation
 
 import (
 	"testing"

projects/gloo/pkg/plugins/utils/validator/options.go

@@ -0,0 +1,41 @@
+package validator
+
+import (
+	"fmt"
+
+	"github.com/solo-io/gloo/pkg/utils"
+	"go.opencensus.io/stats"
+)
+
+type config struct {
+	cacheHits   *stats.Int64Measure
+	cacheMisses *stats.Int64Measure
+	cacheSize   int
+}
+
+type Option func(*config)
+
+func WithCounters(cacheHits, cacheMisses *stats.Int64Measure) Option {
+	return func(s *config) {
+		s.cacheHits = cacheHits
+		s.cacheMisses = cacheMisses
+	}
+}
+
+func WithCacheSize(size int) Option {
+	return func(s *config) {
+		s.cacheSize = size
+	}
+}
+
+func processOptions(name string, options ...Option) *config {
+	cfg := &config{
+		cacheHits:   utils.MakeSumCounter(fmt.Sprintf("gloo.solo.io/%s_validation_cache_hits", name), fmt.Sprintf("The number of cache hits while validating %s config", name)),
+		cacheMisses: utils.MakeSumCounter(fmt.Sprintf("gloo.solo.io/%s_validation_cache_misses", name), fmt.Sprintf("The number of cache misses while validating %s config", name)),
+		cacheSize:   DefaultCacheSize,
+	}
+	for _, option := range options {
+		option(cfg)
+	}
+	return cfg
+}

projects/gloo/pkg/plugins/utils/validator/validator.go

@@ -0,0 +1,86 @@
+package validator
+
+import (
+	"context"
+	"hash"
+
+	"github.com/solo-io/gloo/pkg/utils"
+	"github.com/solo-io/gloo/projects/gloo/pkg/bootstrap"
+	"github.com/solo-io/go-utils/contextutils"
+	"go.opencensus.io/stats"
+	"google.golang.org/protobuf/runtime/protoiface"
+	"k8s.io/utils/lru"
+)
+
+// DefaultCacheSize defines the default size of the LRU cache used by the validator
+const DefaultCacheSize int = 1024
+
+// Validator validates an envoy config by running it by envoy in validate mode. This requires the envoy binary to be present at $ENVOY_BINARY_PATH (defaults to /usr/local/bin/envoy).
+// Results are cached via an LRU cache for performance
+type Validator interface {
+	// ValidateConfig validates the given envoy config and returns any out and error from envoy. Returns nil if the envoy binary is not found.
+	ValidateConfig(ctx context.Context, config HashableProtoMessage) error
+
+	// CacheLength returns the returns the number of items in the cache
+	CacheLength() int
+}
+
+type validator struct {
+	filterName string
+	// lruCache is a map of: (config hash) -> error state
+	// this is usually a typed error but may be an untyped nil interface
+	lruCache *lru.Cache
+	// Counter to increment on cache hits
+	cacheHits *stats.Int64Measure
+	// Counter to increment on cache misses
+	cacheMisses *stats.Int64Measure
+}
+
+// New returns a new Validator
+func New(name string, filterName string, opts ...Option) validator {
+	cfg := processOptions(name, opts...)
+	return validator{
+		filterName:  filterName,
+		lruCache:    lru.New(cfg.cacheSize),
+		cacheHits:   cfg.cacheHits,
+		cacheMisses: cfg.cacheMisses,
+	}
+}
+
+// HashableProtoMessage defines a ProtoMessage that can be hashed. Useful when passing different ProtoMessages objects that need to be hashed.
+type HashableProtoMessage interface {
+	protoiface.MessageV1
+	Hash(hasher hash.Hash64) (uint64, error)
+}
+
+func (v validator) ValidateConfig(ctx context.Context, config HashableProtoMessage) error {
+	hash, err := config.Hash(nil)
+	if err != nil {
+		contextutils.LoggerFrom(ctx).DPanicf("error hashing the config, should never happen: %v", err)
+		return err
+	}
+
+	// This proto has already been validated, return the result
+	if err, ok := v.lruCache.Get(hash); ok {
+		utils.MeasureOne(
+			ctx,
+			v.cacheHits,
+		)
+		// Error may be nil here since it's just the cached result
+		// so return it as a nil err after cast worst case.
+		errCasted, _ := err.(error)
+		return errCasted
+	}
+	utils.MeasureOne(
+		ctx,
+		v.cacheMisses,
+	)
+
+	err = bootstrap.ValidateBootstrap(ctx, v.filterName, config)
+	v.lruCache.Add(hash, err)
+	return err
+}
+
+func (v validator) CacheLength() int {
+	return v.lruCache.Len()
+}

projects/gloo/pkg/plugins/utils/validator/validator_suite_test.go

@@ -0,0 +1,13 @@
+package validator
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestValidator(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Validation Suite")
+}