From dd1badaa0799ca7064b520a6ac2865ed0f546c28 Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Thu, 25 Apr 2024 12:12:18 -0500 Subject: [PATCH 01/30] add status changed predicate --- projects/gateway2/controller/controller.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/projects/gateway2/controller/controller.go b/projects/gateway2/controller/controller.go index ab1e049174a..a75a32eb815 100644 --- a/projects/gateway2/controller/controller.go +++ b/projects/gateway2/controller/controller.go @@ -261,6 +261,7 @@ func (c *controllerBuilder) watchNamespaces(ctx context.Context) error { func (c *controllerBuilder) watchRouteOptions(ctx context.Context) error { err := ctrl.NewControllerManagedBy(c.cfg.Mgr). + WithEventFilter(predicate.GenerationChangedPredicate{}). For(&sologatewayv1.RouteOption{}). Complete(reconcile.Func(c.reconciler.ReconcileRouteOptions)) if err != nil { @@ -271,6 +272,7 @@ func (c *controllerBuilder) watchRouteOptions(ctx context.Context) error { func (c *controllerBuilder) watchVirtualHostOptions(ctx context.Context) error { err := ctrl.NewControllerManagedBy(c.cfg.Mgr). + WithEventFilter(predicate.GenerationChangedPredicate{}). For(&sologatewayv1.VirtualHostOption{}). Complete(reconcile.Func(c.reconciler.ReconcileVirtualHostOptions)) if err != nil { From 7e81455409f2a60de847467c44c902713edf6f1b Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Thu, 25 Apr 2024 12:14:07 -0500 Subject: [PATCH 02/30] wip: validation plumbing; allow validation to create a ggv2 translator --- .../validating_admission_webhook.go | 1 + projects/gateway/pkg/validation/validator.go | 8 ++ projects/gateway2/controller/gw_controller.go | 1 + projects/gateway2/controller/start.go | 78 +++++-------------- projects/gateway2/install.sh | 7 +- .../gateway2/proxy_syncer/proxy_syncer.go | 4 - .../gateway_http_route_translator.go | 1 + .../gloo/pkg/syncer/setup/setup_syncer.go | 67 +++++++++++++--- projects/gloo/pkg/syncer/setup/start_func.go | 28 ++++--- 9 files changed, 105 insertions(+), 90 deletions(-) diff --git a/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook.go b/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook.go index 003884227af..dba66c42763 100644 --- a/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook.go +++ b/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook.go @@ -475,6 +475,7 @@ func (wh *gatewayValidationWebhook) validateList(ctx context.Context, rawJson [] return reports, nil } +// shouldValidateResource determines if a resource should be validated AND populates `resource` (and `oldResource` if applicable) from the objects[s] in the `admissionRequest` func (wh *gatewayValidationWebhook) shouldValidateResource(ctx context.Context, admissionRequest *v1beta1.AdmissionRequest, resource, oldResource resources.HashableResource) (bool, error) { logger := contextutils.LoggerFrom(ctx) diff --git a/projects/gateway/pkg/validation/validator.go b/projects/gateway/pkg/validation/validator.go index d6a80693dac..e38ad499362 100644 --- a/projects/gateway/pkg/validation/validator.go +++ b/projects/gateway/pkg/validation/validator.go @@ -7,8 +7,10 @@ import ( "reflect" "sync" + "github.com/solo-io/gloo/projects/gateway2/extensions" gloov1snap "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/gloosnapshot" "github.com/solo-io/go-utils/hashutils" + "sigs.k8s.io/controller-runtime/pkg/manager" "github.com/hashicorp/go-multierror" errors "github.com/rotisserie/eris" @@ -120,6 +122,8 @@ type validator struct { extensionValidator syncerValidation.Validator allowWarnings bool disableValidationAgainstPreviousState bool + mgr manager.Manager + k8sGwExtensions extensions.K8sGatewayExtensions } type validationOptions struct { @@ -142,6 +146,8 @@ type ValidatorConfig struct { ExtensionValidator syncerValidation.Validator AllowWarnings bool DisableValidationAgainstPreviousState bool + Mgr manager.Manager + K8sGwExtensions extensions.K8sGatewayExtensions } func NewValidator(cfg ValidatorConfig) *validator { @@ -151,6 +157,8 @@ func NewValidator(cfg ValidatorConfig) *validator { translator: cfg.Translator, allowWarnings: cfg.AllowWarnings, disableValidationAgainstPreviousState: cfg.DisableValidationAgainstPreviousState, + mgr: cfg.Mgr, + k8sGwExtensions: cfg.K8sGwExtensions, } } diff --git a/projects/gateway2/controller/gw_controller.go b/projects/gateway2/controller/gw_controller.go index 817603fa27b..10c11ea6a63 100644 --- a/projects/gateway2/controller/gw_controller.go +++ b/projects/gateway2/controller/gw_controller.go @@ -58,6 +58,7 @@ func (r *gatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } log.Info("reconciling gateway", "Gateway", gw.GetObjectMeta()) + // log.Info("reconciling gateway", "GatewayName", gw.GetObjectMeta().GetName(), "GatewayNamespace", gw.GetObjectMeta().GetNamespace()) objs, err := r.deployer.GetObjsToDeploy(ctx, &gw) if err != nil { return ctrl.Result{}, err diff --git a/projects/gateway2/controller/start.go b/projects/gateway2/controller/start.go index b3266c67e82..dbc45b81902 100644 --- a/projects/gateway2/controller/start.go +++ b/projects/gateway2/controller/start.go @@ -5,12 +5,9 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/healthz" - "sigs.k8s.io/controller-runtime/pkg/log/zap" - metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + "sigs.k8s.io/controller-runtime/pkg/manager" apiv1 "sigs.k8s.io/gateway-api/apis/v1" - gatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" - "github.com/solo-io/gloo/projects/gateway2/controller/scheme" "github.com/solo-io/gloo/projects/gateway2/extensions" "github.com/solo-io/gloo/projects/gateway2/proxy_syncer" "github.com/solo-io/gloo/projects/gateway2/secrets" @@ -18,8 +15,6 @@ import ( v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" "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/translator" - "github.com/solo-io/solo-kit/pkg/api/v2/reporter" ) const ( @@ -40,7 +35,7 @@ type StartConfig struct { // ExtensionsFactory is the factory function which will return an extensions.K8sGatewayExtensions // This is responsible for producing the extension points that this controller requires - ExtensionsFactory extensions.K8sGatewayExtensionsFactory + K8sGatewayExtensions extensions.K8sGatewayExtensions // GlooPluginRegistryFactory is the factory function to produce a PluginRegistry // The plugins in this registry are used during the conversion of a Proxy resource into an xDS Snapshot @@ -49,67 +44,34 @@ type StartConfig struct { // ProxyClient is the client that writes Proxy resources into an in-memory cache // This cache is utilized by the debug.ProxyEndpointServer ProxyClient v1.ProxyClient - // RouteOptionClient is the client used for retrieving RouteOption objects within the RouteOptionsPlugin - // NOTE: We may be able to move this entirely to the RouteOptionsPlugin - RouteOptionClient gatewayv1.RouteOptionClient - // StatusReporter is used within any StatusPlugins that must persist a GE-classic style status - StatusReporter reporter.StatusReporter + + InputChannels *proxy_syncer.GatewayInputChannels + + Mgr manager.Manager } // Start runs the controllers responsible for processing the K8s Gateway API objects // It is intended to be run in a goroutine as the function will block until the supplied // context is cancelled func Start(ctx context.Context, cfg StartConfig) error { - var opts []zap.Opts - if cfg.Dev { - setupLog.Info("starting log in dev mode") - opts = append(opts, zap.UseDevMode(true)) - } - ctrl.SetLogger(zap.New(opts...)) - - mgrOpts := ctrl.Options{ - Scheme: scheme.NewScheme(), - PprofBindAddress: "127.0.0.1:9099", - // if you change the port here, also change the port "health" in the helmchart. - HealthProbeBindAddress: ":9093", - Metrics: metricsserver.Options{ - BindAddress: ":9092", - }, - } - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), mgrOpts) - if err != nil { - setupLog.Error(err, "unable to start manager") - return err - } - + // var opts []zap.Opts + // if cfg.Dev { + // setupLog.Info("starting log in dev mode") + // opts = append(opts, zap.UseDevMode(true)) + // } + // ctrl.SetLogger(zap.New(opts...)) + + mgr := cfg.Mgr // TODO: replace this with something that checks that we have xds snapshot ready (or that we don't need one). mgr.AddReadyzCheck("ready-ping", healthz.Ping) - glooTranslator := translator.NewDefaultTranslator( - cfg.Opts.Settings, - cfg.GlooPluginRegistryFactory(ctx)) - - inputChannels := proxy_syncer.NewGatewayInputChannels() - - k8sGwExtensions, err := cfg.ExtensionsFactory(ctx, extensions.K8sGatewayExtensionsFactoryParameters{ - Mgr: mgr, - RouteOptionClient: cfg.RouteOptionClient, - StatusReporter: cfg.StatusReporter, - KickXds: inputChannels.Kick, - }) - if err != nil { - setupLog.Error(err, "unable to create k8s gw extensions") - return err - } - // Create the proxy syncer for the Gateway API resources proxySyncer := proxy_syncer.NewProxySyncer( wellknown.GatewayControllerName, cfg.Opts.WriteNamespace, - glooTranslator, - inputChannels, + cfg.InputChannels, mgr, - k8sGwExtensions, + cfg.K8sGatewayExtensions, cfg.ProxyClient, ) if err := mgr.Add(proxySyncer); err != nil { @@ -124,15 +86,15 @@ func Start(ctx context.Context, cfg StartConfig) error { AutoProvision: AutoProvision, ControlPlane: cfg.Opts.ControlPlane, IstioValues: cfg.Opts.GlooGateway.IstioValues, - Kick: inputChannels.Kick, - Extensions: k8sGwExtensions, + Kick: cfg.InputChannels.Kick, + Extensions: cfg.K8sGatewayExtensions, } - if err = NewBaseGatewayController(ctx, gwCfg); err != nil { + if err := NewBaseGatewayController(ctx, gwCfg); err != nil { setupLog.Error(err, "unable to create controller") return err } - if err = secrets.NewSecretsController(ctx, mgr, inputChannels); err != nil { + if err := secrets.NewSecretsController(ctx, mgr, cfg.InputChannels); err != nil { setupLog.Error(err, "unable to create controller") return err } diff --git a/projects/gateway2/install.sh b/projects/gateway2/install.sh index 5d2981e2ec9..49fd8df2e8b 100755 --- a/projects/gateway2/install.sh +++ b/projects/gateway2/install.sh @@ -5,7 +5,12 @@ set -eux ./projects/gateway2/kind.sh +kubectl apply -f https://github.com/kubernetes-sigs/gateway-api/releases/download/v1.0.0/standard-install.yaml + helm upgrade --install --create-namespace \ --namespace gloo-system gloo \ ./_test/gloo-1.0.0-ci1.tgz \ - -f ./projects/gateway2/tests/conformance/test-values.yaml + -f - < Date: Thu, 25 Apr 2024 12:30:59 -0500 Subject: [PATCH 03/30] fix merge --- projects/gloo/pkg/syncer/setup/start_func.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/projects/gloo/pkg/syncer/setup/start_func.go b/projects/gloo/pkg/syncer/setup/start_func.go index 61184b0458e..ce1850efc78 100644 --- a/projects/gloo/pkg/syncer/setup/start_func.go +++ b/projects/gloo/pkg/syncer/setup/start_func.go @@ -9,13 +9,9 @@ import ( "github.com/solo-io/go-utils/contextutils" "github.com/solo-io/gloo/projects/gateway2/controller" -<<<<<<< HEAD "github.com/solo-io/gloo/projects/gateway2/extensions" -======= ->>>>>>> main "github.com/solo-io/gloo/projects/gateway2/proxy_syncer" v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" - api "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/enterprise/options/extauth/v1" "github.com/solo-io/gloo/projects/gloo/pkg/bootstrap" "github.com/solo-io/gloo/projects/gloo/pkg/debug" ) @@ -75,8 +71,8 @@ func K8sGatewayControllerStartFunc( Mgr: mgr, InputChannels: inputChannels, - ProxyClient: proxyClient, - QueueStatusForProxies: queueStatusForProxies, + ProxyClient: proxyClient, + QueueStatusForProxies: queueStatusForProxies, // Useful for development purposes // At the moment, this is not tied to any user-facing API From 6792cff956d6fb8c982631531b765ac3219058d7 Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Wed, 1 May 2024 12:12:50 -0500 Subject: [PATCH 04/30] wip: ggv2 validator impl --- projects/gateway/pkg/validation/validator.go | 124 +++++++--- .../gateway/pkg/validation/validator_test.go | 21 ++ .../controller/controller_suite_test.go | 3 +- projects/gateway2/deployer/deployer_test.go | 21 +- ...mple-http-route-with-attached-options.yaml | 3 + projects/gateway2/extensions/extensions.go | 18 +- .../testutils/validation_test_utils.go | 204 ++++++++++++++++ projects/gateway2/validation/validator.go | 85 +++++++ .../validation/validator_suite_test.go | 13 ++ .../gateway2/validation/validator_test.go | 217 ++++++++++++++++++ .../gloo/pkg/syncer/setup/setup_syncer.go | 13 +- 11 files changed, 657 insertions(+), 65 deletions(-) create mode 100644 projects/gateway2/validation/testutils/validation_test_utils.go create mode 100644 projects/gateway2/validation/validator.go create mode 100644 projects/gateway2/validation/validator_suite_test.go create mode 100644 projects/gateway2/validation/validator_test.go diff --git a/projects/gateway/pkg/validation/validator.go b/projects/gateway/pkg/validation/validator.go index 4df468aab46..3439fb5cce6 100644 --- a/projects/gateway/pkg/validation/validator.go +++ b/projects/gateway/pkg/validation/validator.go @@ -7,16 +7,16 @@ import ( "reflect" "sync" - "github.com/solo-io/gloo/projects/gateway2/extensions" gloov1snap "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/gloosnapshot" "github.com/solo-io/go-utils/hashutils" - "sigs.k8s.io/controller-runtime/pkg/manager" "github.com/hashicorp/go-multierror" errors "github.com/rotisserie/eris" utils2 "github.com/solo-io/gloo/pkg/utils" + sologatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" "github.com/solo-io/gloo/projects/gateway/pkg/translator" "github.com/solo-io/gloo/projects/gateway/pkg/utils" + k8svalidation "github.com/solo-io/gloo/projects/gateway2/validation" "github.com/solo-io/gloo/projects/gloo/pkg/api/grpc/validation" gloov1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" syncerValidation "github.com/solo-io/gloo/projects/gloo/pkg/syncer/validation" @@ -122,8 +122,7 @@ type validator struct { extensionValidator syncerValidation.Validator allowWarnings bool disableValidationAgainstPreviousState bool - mgr manager.Manager - k8sGwExtensions extensions.K8sGatewayExtensions + k8sGatewayValidator k8svalidation.ValidationHelper } type validationOptions struct { @@ -146,8 +145,7 @@ type ValidatorConfig struct { ExtensionValidator syncerValidation.Validator AllowWarnings bool DisableValidationAgainstPreviousState bool - Mgr manager.Manager - K8sGwExtensions extensions.K8sGatewayExtensions + K8sGatewayValidator k8svalidation.ValidationHelper } func NewValidator(cfg ValidatorConfig) *validator { @@ -157,8 +155,7 @@ func NewValidator(cfg ValidatorConfig) *validator { translator: cfg.Translator, allowWarnings: cfg.AllowWarnings, disableValidationAgainstPreviousState: cfg.DisableValidationAgainstPreviousState, - mgr: cfg.Mgr, - k8sGwExtensions: cfg.K8sGwExtensions, + k8sGatewayValidator: cfg.K8sGatewayValidator, } } @@ -253,6 +250,63 @@ type validationOutput struct { err error } +// the returned error is a multierror that may contain errors from several Edge Proxies, although this depends on collectAllErrors +func (v *validator) translateGlooEdgeProxies( + ctx context.Context, + snapshot *gloov1snap.ApiSnapshot, + collectAllErrors bool, +) ([]*gloov1.Proxy, error) { + var ( + proxies []*gloov1.Proxy + errs error + ) + + gatewaysByProxy := utils.SortedGatewaysByProxyName(snapshot.Gateways) + + // translate all the proxies + for _, gatewayAndProxyName := range gatewaysByProxy { + proxyName := gatewayAndProxyName.Name + gatewayList := gatewayAndProxyName.Gateways + + // Translate the proxy and process the errors + proxy, reports := v.translator.Translate(ctx, proxyName, snapshot, gatewayList) + + err := v.getErrorsFromResourceReports(reports) + + if err != nil { + err = errors.Wrapf(err, couldNotRenderProxy) + errs = multierror.Append(errs, err) + + if !collectAllErrors { + continue + } + } + + // A nil proxy may have been returned if 0 listeners were created + // continue here even if collecting all errors, because the proxy is nil and there is nothing to validate + if proxy == nil { + continue + } + proxies = append(proxies, proxy) + } + + return proxies, errs +} + +func isK8sGatewayProxy(res resources.Resource) bool { + if _, ok := res.(*sologatewayv1.RouteOption); ok { + return true + } + return false +} + +func (v *validator) translateK8sGatewayProxies( + ctx context.Context, + res resources.Resource, +) ([]*gloov1.Proxy, error) { + return v.k8sGatewayValidator.TranslateK8sGatewayProxies(ctx, res) +} + // validateProxiesAndExtensions validates a snapshot against the Gloo and Gateway Translations. This was removed from the // main validation loop to allow it to be re-run against the original snapshot. The reason for this second validaiton run is to allow // the deletion of secrets, but only if they are not in use by the snapshot. This function does not know about @@ -291,34 +345,14 @@ func (v *validator) validateProxiesAndExtensions(ctx context.Context, snapshot * err error ) - gatewaysByProxy := utils.SortedGatewaysByProxyName(snapshot.Gateways) - - // translate all the proxies - for _, gatewayAndProxyName := range gatewaysByProxy { - proxyName := gatewayAndProxyName.Name - gatewayList := gatewayAndProxyName.Gateways - - // Translate the proxy and process the errors - proxy, reports := v.translator.Translate(ctx, proxyName, snapshot, gatewayList) - - err := v.getErrorsFromResourceReports(reports) - - if err != nil { - err = errors.Wrapf(err, couldNotRenderProxy) - errs = multierror.Append(errs, err) - - if !opts.collectAllErrors { - continue - } - } - - // A nil proxy may have been returned if 0 listeners were created - // continue here even if collecting all errors, because the proxy is nil and there is nothing to validate - if proxy == nil { - continue - } - proxies = append(proxies, proxy) + if isK8sGatewayProxy(opts.Resource) { + proxies, errs = v.translateK8sGatewayProxies(ctx, opts.Resource) + } else { + proxies, errs = v.translateGlooEdgeProxies(ctx, snapshot, opts.collectAllErrors) + } + // Now perform gloo validation for all the Proxies + for _, proxy := range proxies { // Validate the proxy with the Gloo validator // This validation also attempts to modify the snapshot, so when validating the unmodified snapshot a nil resource is passed in so no modifications are made resourceToModify := opts.Resource @@ -688,7 +722,15 @@ func findBreakingErrors(errs error) bool { // ValidateDeletedGvk will validate a deletion of a resource, as long as it is supported, against the Gateway and Gloo Translations. func (v *validator) ValidateDeletedGvk(ctx context.Context, gvk schema.GroupVersionKind, resource resources.Resource, dryRun bool) error { - _, err := v.validateResource(&validationOptions{Ctx: ctx, Resource: resource, Gvk: gvk, Delete: true, DryRun: dryRun, AcquireLock: true}) + opts := &validationOptions{ + Ctx: ctx, + Resource: resource, + Gvk: gvk, + Delete: true, + DryRun: dryRun, + AcquireLock: true, + } + _, err := v.validateResource(opts) return err } @@ -700,7 +742,15 @@ func (v *validator) ValidateModifiedGvk(ctx context.Context, gvk schema.GroupVer func (v *validator) validateModifiedResource(ctx context.Context, gvk schema.GroupVersionKind, resource resources.Resource, dryRun, acquireLock bool) (*Reports, error) { var reports *Reports - reports, err := v.validateResource(&validationOptions{Ctx: ctx, Resource: resource, Gvk: gvk, Delete: false, DryRun: dryRun, AcquireLock: acquireLock}) + opts := &validationOptions{ + Ctx: ctx, + Resource: resource, + Gvk: gvk, + Delete: false, + DryRun: dryRun, + AcquireLock: acquireLock, + } + reports, err := v.validateResource(opts) if err != nil { return reports, &multierror.Error{Errors: []error{errors.Wrapf(err, "Validating %T failed", resource)}} } diff --git a/projects/gateway/pkg/validation/validator_test.go b/projects/gateway/pkg/validation/validator_test.go index b0213d568af..68a483ceff0 100644 --- a/projects/gateway/pkg/validation/validator_test.go +++ b/projects/gateway/pkg/validation/validator_test.go @@ -14,6 +14,7 @@ import ( "github.com/solo-io/gloo/pkg/utils" v1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" "github.com/solo-io/gloo/projects/gateway/pkg/translator" + validationtestutils "github.com/solo-io/gloo/projects/gateway2/validation/testutils" "github.com/solo-io/gloo/projects/gloo/pkg/api/grpc/validation" gloov1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" gloov1snap "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/gloosnapshot" @@ -461,6 +462,26 @@ var _ = Describe("Validator", func() { }) + Context("validating route option in ggv2", func() { + Context("proxy validation accepted", func() { + FIt("accepts the rtopt", func() { + ctx := context.Background() + + v.k8sGatewayValidator = validationtestutils.BuildValidationHelper() + + v.glooValidator = ValidateAccept + // simple snapshot is fine, for kube gateway validation we don't need the RouteOption to be part of the snap + snap := samples.SimpleGlooSnapshot(ns) + err := v.Sync(context.TODO(), snap) + Expect(err).NotTo(HaveOccurred()) + + reports, err := v.ValidateModifiedGvk(ctx, v1.RouteOptionGVK, validationtestutils.AttachedInternal(), false) + Expect(err).NotTo(HaveOccurred()) + Expect(*(reports.ProxyReports)).To(HaveLen(1)) + }) + }) + }) + Context("validating a route table", func() { Context("proxy validation accepted", func() { It("accepts the rt", func() { diff --git a/projects/gateway2/controller/controller_suite_test.go b/projects/gateway2/controller/controller_suite_test.go index fb2f39ad57e..1d8f9350851 100644 --- a/projects/gateway2/controller/controller_suite_test.go +++ b/projects/gateway2/controller/controller_suite_test.go @@ -99,7 +99,8 @@ var _ = BeforeSuite(func() { var gatewayClassObjName api.ObjectName = api.ObjectName(gatewayClassName) exts, err := extensions.NewK8sGatewayExtensions(ctx, extensions.K8sGatewayExtensionsFactoryParameters{ - Mgr: mgr, + Cl: k8sClient, + Scheme: scheme, }) Expect(err).ToNot(HaveOccurred()) cfg := controller.GatewayConfig{ diff --git a/projects/gateway2/deployer/deployer_test.go b/projects/gateway2/deployer/deployer_test.go index fee5d4dee7c..68bbf8627b7 100644 --- a/projects/gateway2/deployer/deployer_test.go +++ b/projects/gateway2/deployer/deployer_test.go @@ -26,8 +26,6 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/yaml" - "k8s.io/client-go/rest" - ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" api "sigs.k8s.io/gateway-api/apis/v1" @@ -121,11 +119,8 @@ var _ = Describe("Deployer", func() { ControllerName: wellknown.GatewayControllerName, }, } - mgr, err := ctrl.NewManager(&rest.Config{}, ctrl.Options{}) - Expect(err).NotTo(HaveOccurred()) - k8sGatewayExt, err = extensions.NewK8sGatewayExtensions(context.TODO(), extensions.K8sGatewayExtensionsFactoryParameters{ - Mgr: mgr, - }) + var err error + k8sGatewayExt, err = extensions.NewK8sGatewayExtensions(context.TODO(), extensions.K8sGatewayExtensionsFactoryParameters{}) Expect(err).NotTo(HaveOccurred()) }) @@ -151,11 +146,7 @@ var _ = Describe("Deployer", func() { }) It("support segmenting by release", func() { - mgr, err := ctrl.NewManager(&rest.Config{}, ctrl.Options{}) - Expect(err).NotTo(HaveOccurred()) - k8sGatewayExt, err := extensions.NewK8sGatewayExtensions(context.TODO(), extensions.K8sGatewayExtensionsFactoryParameters{ - Mgr: mgr, - }) + k8sGatewayExt, err := extensions.NewK8sGatewayExtensions(context.TODO(), extensions.K8sGatewayExtensionsFactoryParameters{}) Expect(err).NotTo(HaveOccurred()) d1, err := deployer.NewDeployer(newFakeClientWithObjs(gwc), &deployer.Inputs{ @@ -254,11 +245,7 @@ var _ = Describe("Deployer", func() { var ( defaultGwpName = "default-gateway-params" defaultDeployerInputs = func() *deployer.Inputs { - mgr, err := ctrl.NewManager(&rest.Config{}, ctrl.Options{}) - Expect(err).NotTo(HaveOccurred()) - k8sGatewayExt, err := extensions.NewK8sGatewayExtensions(context.TODO(), extensions.K8sGatewayExtensionsFactoryParameters{ - Mgr: mgr, - }) + k8sGatewayExt, err := extensions.NewK8sGatewayExtensions(context.TODO(), extensions.K8sGatewayExtensionsFactoryParameters{}) Expect(err).NotTo(HaveOccurred()) return &deployer.Inputs{ ControllerName: wellknown.GatewayControllerName, diff --git a/projects/gateway2/examples/example-http-route-with-attached-options.yaml b/projects/gateway2/examples/example-http-route-with-attached-options.yaml index 67e383348aa..f0dc6eb4a6e 100644 --- a/projects/gateway2/examples/example-http-route-with-attached-options.yaml +++ b/projects/gateway2/examples/example-http-route-with-attached-options.yaml @@ -17,6 +17,7 @@ spec: value: /timeout backendRefs: - name: example-svc + namespace: nginx port: 8080 --- apiVersion: gateway.solo.io/v1 @@ -39,6 +40,7 @@ apiVersion: v1 kind: Service metadata: name: example-svc + namespace: nginx spec: selector: app.kubernetes.io/name: nginx @@ -51,6 +53,7 @@ apiVersion: v1 kind: Pod metadata: name: nginx + namespace: nginx labels: app.kubernetes.io/name: nginx spec: diff --git a/projects/gateway2/extensions/extensions.go b/projects/gateway2/extensions/extensions.go index cfded6c3663..3fd5da3a6a7 100644 --- a/projects/gateway2/extensions/extensions.go +++ b/projects/gateway2/extensions/extensions.go @@ -4,13 +4,14 @@ import ( "context" v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/enterprise/options/extauth/v1" + "k8s.io/apimachinery/pkg/runtime" "github.com/solo-io/gloo/pkg/version" gatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" "github.com/solo-io/gloo/projects/gateway2/query" "github.com/solo-io/gloo/projects/gateway2/translator/plugins/registry" "github.com/solo-io/solo-kit/pkg/api/v2/reporter" - controllerruntime "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" ) // K8sGatewayExtensions is responsible for providing implementations for translation utilities @@ -25,7 +26,8 @@ type K8sGatewayExtensions interface { // K8sGatewayExtensionsFactoryParameters contains the parameters required to start Gloo K8s Gateway Extensions (including Translator Plugins) type K8sGatewayExtensionsFactoryParameters struct { - Mgr controllerruntime.Manager + Cl client.Client + Scheme *runtime.Scheme AuthConfigClient v1.AuthConfigClient RouteOptionClient gatewayv1.RouteOptionClient StatusReporter reporter.StatusReporter @@ -44,14 +46,16 @@ func NewK8sGatewayExtensions( params K8sGatewayExtensionsFactoryParameters, ) (K8sGatewayExtensions, error) { return &k8sGatewayExtensions{ - params.Mgr, + params.Cl, + params.Scheme, params.RouteOptionClient, params.StatusReporter, }, nil } type k8sGatewayExtensions struct { - mgr controllerruntime.Manager + cl client.Client + scheme *runtime.Scheme routeOptionClient gatewayv1.RouteOptionClient statusReporter reporter.StatusReporter } @@ -59,12 +63,12 @@ type k8sGatewayExtensions struct { // CreatePluginRegistry returns the PluginRegistry func (e *k8sGatewayExtensions) CreatePluginRegistry(_ context.Context) registry.PluginRegistry { queries := query.NewData( - e.mgr.GetClient(), - e.mgr.GetScheme(), + e.cl, + e.scheme, ) plugins := registry.BuildPlugins( queries, - e.mgr.GetClient(), + e.cl, e.routeOptionClient, e.statusReporter, ) diff --git a/projects/gateway2/validation/testutils/validation_test_utils.go b/projects/gateway2/validation/testutils/validation_test_utils.go new file mode 100644 index 00000000000..6bd5deb2616 --- /dev/null +++ b/projects/gateway2/validation/testutils/validation_test_utils.go @@ -0,0 +1,204 @@ +package testutils + +import ( + "context" + + "google.golang.org/protobuf/types/known/wrapperspb" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/solo-io/gloo/pkg/utils/statusutils" + sologatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" + solokubev1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1/kube/apis/gateway.solo.io/v1" + "github.com/solo-io/gloo/projects/gateway2/controller/scheme" + "github.com/solo-io/gloo/projects/gateway2/extensions" + "github.com/solo-io/gloo/projects/gateway2/proxy_syncer" + gwquery "github.com/solo-io/gloo/projects/gateway2/query" + rtoptquery "github.com/solo-io/gloo/projects/gateway2/translator/plugins/routeoptions/query" + "github.com/solo-io/gloo/projects/gateway2/translator/testutils" + "github.com/solo-io/gloo/projects/gateway2/validation" + "github.com/solo-io/gloo/projects/gateway2/wellknown" + v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" + extauth "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/enterprise/options/extauth/v1" + "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/faultinjection" + corev1 "github.com/solo-io/skv2/pkg/api/core.skv2.solo.io/v1" + "github.com/solo-io/solo-kit/pkg/api/v1/clients" + "github.com/solo-io/solo-kit/pkg/api/v1/clients/factory" + "github.com/solo-io/solo-kit/pkg/api/v1/clients/memory" + "github.com/solo-io/solo-kit/pkg/api/v1/resources/core" + "github.com/solo-io/solo-kit/pkg/api/v2/reporter" + + k8scorev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gwv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +func BuildValidationHelper() validation.ValidationHelper { + var ( + ctx context.Context + sch *runtime.Scheme + authConfigClient extauth.AuthConfigClient + routeOptionClient sologatewayv1.RouteOptionClient + statusReporter reporter.StatusReporter + inputChannels *proxy_syncer.GatewayInputChannels + ) + + ctx = context.Background() + + sch = scheme.NewScheme() + resourceClientFactory := &factory.MemoryResourceClientFactory{ + Cache: memory.NewInMemoryResourceCache(), + } + + routeOptionClient, _ = sologatewayv1.NewRouteOptionClient(ctx, resourceClientFactory) + authConfigClient, _ = extauth.NewAuthConfigClient(ctx, resourceClientFactory) + statusClient := statusutils.GetStatusClientForNamespace("gloo-system") + + statusReporter = reporter.NewReporter("gloo-kube-gateway", statusClient, routeOptionClient.BaseClient()) + + inputChannels = proxy_syncer.NewGatewayInputChannels() + + deps := []client.Object{svc(), gw(), httpRoute(), attachedRouteOption()} + routeOptionClient.Write(AttachedInternal(), clients.WriteOpts{}) + fakeClient := testutils.BuildIndexedFakeClient(deps, gwquery.IterateIndices, rtoptquery.IterateIndices) + gwQueries := testutils.BuildGatewayQueriesWithClient(fakeClient) + + k8sGwExtensions, _ := extensions.NewK8sGatewayExtensions(ctx, extensions.K8sGatewayExtensionsFactoryParameters{ + Cl: fakeClient, + Scheme: sch, + AuthConfigClient: authConfigClient, + RouteOptionClient: routeOptionClient, + StatusReporter: statusReporter, + KickXds: inputChannels.Kick, + }) + + validator := validation.ValidationHelper{ + K8sGwExtensions: k8sGwExtensions, + GatewayQueries: gwQueries, + Cl: fakeClient, + } + return validator +} + +func nsPtr(s string) *gwv1.Namespace { + var ns gwv1.Namespace = gwv1.Namespace(s) + return &ns +} + +func portNumPtr(n int32) *gwv1.PortNumber { + var pn gwv1.PortNumber = gwv1.PortNumber(n) + return &pn +} + +func svc() *k8scorev1.Service { + return &k8scorev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "my-svc", + }, + Spec: k8scorev1.ServiceSpec{ + Ports: []k8scorev1.ServicePort{ + { + Port: 8080, + }, + }, + }, + } +} + +func httpRoute() *gwv1.HTTPRoute { + return &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-route", + Namespace: "default", + }, + Spec: gwv1.HTTPRouteSpec{ + CommonRouteSpec: gwv1.CommonRouteSpec{ + ParentRefs: []gwv1.ParentReference{ + { + Name: "my-gw", + Namespace: nsPtr("default"), + }, + }, + }, + Hostnames: []gwv1.Hostname{ + gwv1.Hostname("example.com"), + }, + Rules: []gwv1.HTTPRouteRule{ + { + BackendRefs: []gwv1.HTTPBackendRef{ + { + BackendRef: gwv1.BackendRef{ + BackendObjectReference: gwv1.BackendObjectReference{ + Name: "my-svc", + Port: portNumPtr(8080), + }, + }, + }, + }, + }, + }, + }, + } +} + +func gw() *gwv1.Gateway { + return &gwv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-gw", + Namespace: "default", + }, + Spec: gwv1.GatewaySpec{ + Listeners: []gwv1.Listener{ + { + Name: "my-http-listener", + Port: gwv1.PortNumber(8080), + Protocol: gwv1.HTTPProtocolType, + // AllowedRoutes: &gwv1.AllowedRoutes{ + // Namespaces: &gwv1.RouteNamespaces{ + // gw + // }, + // }, + }, + }, + }, + } +} + +func attachedRouteOption() *solokubev1.RouteOption { + now := metav1.Now() + return &solokubev1.RouteOption{ + TypeMeta: metav1.TypeMeta{ + Kind: sologatewayv1.RouteOptionGVK.Kind, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "policy", + Namespace: "default", + CreationTimestamp: now, + }, + Spec: *AttachedInternal(), + } +} + +func AttachedInternal() *sologatewayv1.RouteOption { + return &sologatewayv1.RouteOption{ + Metadata: &core.Metadata{ + Name: "policy", + Namespace: "default", + }, + TargetRef: &corev1.PolicyTargetReference{ + Group: gwv1.GroupVersion.Group, + Kind: wellknown.HTTPRouteKind, + Name: "my-route", + Namespace: wrapperspb.String("default"), + }, + Options: &v1.RouteOptions{ + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + Percentage: 4.19, + HttpStatus: 500, + }, + }, + }, + } +} diff --git a/projects/gateway2/validation/validator.go b/projects/gateway2/validation/validator.go new file mode 100644 index 00000000000..7af7cf2f31e --- /dev/null +++ b/projects/gateway2/validation/validator.go @@ -0,0 +1,85 @@ +package validation + +import ( + "context" + + sologatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" + "github.com/solo-io/gloo/projects/gateway2/extensions" + "github.com/solo-io/gloo/projects/gateway2/query" + "github.com/solo-io/gloo/projects/gateway2/reports" + "github.com/solo-io/gloo/projects/gateway2/translator" + gloov1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" + "github.com/solo-io/solo-kit/pkg/api/v1/resources" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + gwv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +// contains data needed to bootstrap a GW2 translator +type ValidationHelper struct { + K8sGwExtensions extensions.K8sGatewayExtensions + GatewayQueries query.GatewayQueries + Cl client.Client +} + +type ValidatorClient struct { +} + +func (v *ValidatorClient) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + return nil +} + +func (v *ValidationHelper) TranslateK8sGatewayProxies(ctx context.Context, res resources.Resource) ([]*gloov1.Proxy, error) { + // we need to find the Gateway associated with the resource + rtOpt, ok := res.(*sologatewayv1.RouteOption) + if !ok { + panic("uh oh") + } + + // first find the target HTTPRoute + var hr gwv1.HTTPRoute + targetRef := rtOpt.GetTargetRef() + hrnn := types.NamespacedName{ + Namespace: targetRef.GetNamespace().GetValue(), + Name: targetRef.GetName(), + } + err := v.Cl.Get(ctx, hrnn, &hr, &client.GetOptions{}) + if err != nil { + panic("err getting") + } + + // now we directly get the parentRefs (which are Gateways!) + var gws []gwv1.Gateway + for _, pr := range hr.Spec.ParentRefs { + var gw gwv1.Gateway + gwnn := types.NamespacedName{ + Namespace: string(*pr.Namespace), + Name: string(pr.Name), + } + err := v.Cl.Get(ctx, gwnn, &gw, &client.GetOptions{}) + if err != nil { + panic("err getting GW") + } + gws = append(gws, gw) + } + + // create the plugins and translator + plugins := v.K8sGwExtensions.CreatePluginRegistry(ctx) + t := translator.NewTranslator(v.GatewayQueries, plugins) + rm := reports.NewReportMap() + r := reports.NewReporter(&rm) + + // translate all the gateways and collect the output Proxies + var proxies []*gloov1.Proxy + for _, gw := range gws { + proxy := t.TranslateProxy(ctx, &gw, "gloo-system", r) + if proxy == nil || len(proxy.GetListeners()) == 0 { + continue + } + proxies = append(proxies, proxy) + } + + // we currently don't want to fail validation for any K8s Gateway-specific error conditions, as the behavior and status reporting + // requirements for these scenarios are built into the spec; let's always return a nil error until that decision changes + return proxies, nil +} diff --git a/projects/gateway2/validation/validator_suite_test.go b/projects/gateway2/validation/validator_suite_test.go new file mode 100644 index 00000000000..61168204bed --- /dev/null +++ b/projects/gateway2/validation/validator_suite_test.go @@ -0,0 +1,13 @@ +package validation_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestValidator(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Validator Suite") +} diff --git a/projects/gateway2/validation/validator_test.go b/projects/gateway2/validation/validator_test.go new file mode 100644 index 00000000000..6ffa6c2dd4f --- /dev/null +++ b/projects/gateway2/validation/validator_test.go @@ -0,0 +1,217 @@ +package validation_test + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + "google.golang.org/protobuf/types/known/wrapperspb" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + // . "github.com/onsi/gomega" + + "github.com/solo-io/gloo/pkg/utils/statusutils" + sologatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" + solokubev1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1/kube/apis/gateway.solo.io/v1" + "github.com/solo-io/gloo/projects/gateway2/controller/scheme" + "github.com/solo-io/gloo/projects/gateway2/extensions" + "github.com/solo-io/gloo/projects/gateway2/proxy_syncer" + gwquery "github.com/solo-io/gloo/projects/gateway2/query" + rtoptquery "github.com/solo-io/gloo/projects/gateway2/translator/plugins/routeoptions/query" + "github.com/solo-io/gloo/projects/gateway2/translator/testutils" + "github.com/solo-io/gloo/projects/gateway2/validation" + "github.com/solo-io/gloo/projects/gateway2/wellknown" + v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" + extauth "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/enterprise/options/extauth/v1" + "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/faultinjection" + corev1 "github.com/solo-io/skv2/pkg/api/core.skv2.solo.io/v1" + "github.com/solo-io/solo-kit/pkg/api/v1/clients" + "github.com/solo-io/solo-kit/pkg/api/v1/clients/factory" + "github.com/solo-io/solo-kit/pkg/api/v1/clients/memory" + "github.com/solo-io/solo-kit/pkg/api/v1/resources/core" + "github.com/solo-io/solo-kit/pkg/api/v2/reporter" + + k8scorev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gwv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +var _ = Describe("RouteOptionsPlugin", func() { + var ( + ctx context.Context + cancel context.CancelFunc + sch *runtime.Scheme + authConfigClient extauth.AuthConfigClient + routeOptionClient sologatewayv1.RouteOptionClient + statusReporter reporter.StatusReporter + inputChannels *proxy_syncer.GatewayInputChannels + ) + + BeforeEach(func() { + ctx, cancel = context.WithCancel(context.Background()) + + sch = scheme.NewScheme() + resourceClientFactory := &factory.MemoryResourceClientFactory{ + Cache: memory.NewInMemoryResourceCache(), + } + + routeOptionClient, _ = sologatewayv1.NewRouteOptionClient(ctx, resourceClientFactory) + authConfigClient, _ = extauth.NewAuthConfigClient(ctx, resourceClientFactory) + statusClient := statusutils.GetStatusClientForNamespace("gloo-system") + + statusReporter = reporter.NewReporter("gloo-kube-gateway", statusClient, routeOptionClient.BaseClient()) + + inputChannels = proxy_syncer.NewGatewayInputChannels() + }) + + AfterEach(func() { + cancel() + }) + + It("validates a RouteOption", func() { + deps := []client.Object{svc(), gw(), httpRoute(), attachedRouteOption()} + routeOptionClient.Write(attachedInternal(), clients.WriteOpts{}) + fakeClient := testutils.BuildIndexedFakeClient(deps, gwquery.IterateIndices, rtoptquery.IterateIndices) + gwQueries := testutils.BuildGatewayQueriesWithClient(fakeClient) + + k8sGwExtensions, _ := extensions.NewK8sGatewayExtensions(ctx, extensions.K8sGatewayExtensionsFactoryParameters{ + Cl: fakeClient, + Scheme: sch, + AuthConfigClient: authConfigClient, + RouteOptionClient: routeOptionClient, + StatusReporter: statusReporter, + KickXds: inputChannels.Kick, + }) + + rtOpt := attachedInternal() + validator := validation.ValidationHelper{ + K8sGwExtensions: k8sGwExtensions, + GatewayQueries: gwQueries, + Cl: fakeClient, + } + validator.TranslateK8sGatewayProxies(ctx, rtOpt) + }) +}) + +func nsPtr(s string) *gwv1.Namespace { + var ns gwv1.Namespace = gwv1.Namespace(s) + return &ns +} + +func portNumPtr(n int32) *gwv1.PortNumber { + var pn gwv1.PortNumber = gwv1.PortNumber(n) + return &pn +} + +func svc() *k8scorev1.Service { + return &k8scorev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "my-svc", + }, + Spec: k8scorev1.ServiceSpec{ + Ports: []k8scorev1.ServicePort{ + { + Port: 8080, + }, + }, + }, + } +} + +func httpRoute() *gwv1.HTTPRoute { + return &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-route", + Namespace: "default", + }, + Spec: gwv1.HTTPRouteSpec{ + CommonRouteSpec: gwv1.CommonRouteSpec{ + ParentRefs: []gwv1.ParentReference{ + { + Name: "my-gw", + Namespace: nsPtr("default"), + }, + }, + }, + Hostnames: []gwv1.Hostname{ + gwv1.Hostname("example.com"), + }, + Rules: []gwv1.HTTPRouteRule{ + { + BackendRefs: []gwv1.HTTPBackendRef{ + { + BackendRef: gwv1.BackendRef{ + BackendObjectReference: gwv1.BackendObjectReference{ + Name: "my-svc", + Port: portNumPtr(8080), + }, + }, + }, + }, + }, + }, + }, + } +} + +func gw() *gwv1.Gateway { + return &gwv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-gw", + Namespace: "default", + }, + Spec: gwv1.GatewaySpec{ + Listeners: []gwv1.Listener{ + { + Name: "my-http-listener", + Port: gwv1.PortNumber(8080), + Protocol: gwv1.HTTPProtocolType, + // AllowedRoutes: &gwv1.AllowedRoutes{ + // Namespaces: &gwv1.RouteNamespaces{ + // gw + // }, + // }, + }, + }, + }, + } +} + +func attachedRouteOption() *solokubev1.RouteOption { + now := metav1.Now() + return &solokubev1.RouteOption{ + TypeMeta: metav1.TypeMeta{ + Kind: sologatewayv1.RouteOptionGVK.Kind, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "policy", + Namespace: "default", + CreationTimestamp: now, + }, + Spec: *attachedInternal(), + } +} + +func attachedInternal() *sologatewayv1.RouteOption { + return &sologatewayv1.RouteOption{ + Metadata: &core.Metadata{ + Name: "policy", + Namespace: "default", + }, + TargetRef: &corev1.PolicyTargetReference{ + Group: gwv1.GroupVersion.Group, + Kind: wellknown.HTTPRouteKind, + Name: "my-route", + Namespace: wrapperspb.String("default"), + }, + Options: &v1.RouteOptions{ + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + Percentage: 4.19, + HttpStatus: 500, + }, + }, + }, + } +} diff --git a/projects/gloo/pkg/syncer/setup/setup_syncer.go b/projects/gloo/pkg/syncer/setup/setup_syncer.go index 4db7304d828..599c1af238f 100644 --- a/projects/gloo/pkg/syncer/setup/setup_syncer.go +++ b/projects/gloo/pkg/syncer/setup/setup_syncer.go @@ -55,7 +55,9 @@ import ( "github.com/solo-io/gloo/projects/gateway2/controller/scheme" k8sgwextensions "github.com/solo-io/gloo/projects/gateway2/extensions" "github.com/solo-io/gloo/projects/gateway2/proxy_syncer" + "github.com/solo-io/gloo/projects/gateway2/query" "github.com/solo-io/gloo/projects/gateway2/status" + k8svalidation "github.com/solo-io/gloo/projects/gateway2/validation" "github.com/solo-io/gloo/projects/gloo/constants" rlv1alpha1 "github.com/solo-io/gloo/projects/gloo/pkg/api/external/solo/ratelimit" v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" @@ -887,7 +889,8 @@ func RunGlooWithExtensions(opts bootstrap.Opts, extensions Extensions) error { statusReporter := reporter.NewReporter("gloo-kube-gateway", statusClient, routeOptionClient.BaseClient()) inputChannels := proxy_syncer.NewGatewayInputChannels() k8sGwExtensions, err := extensions.K8sGatewayExtensionsFactory(watchOpts.Ctx, k8sgwextensions.K8sGatewayExtensionsFactoryParameters{ - Mgr: mgr, + Cl: mgr.GetClient(), + Scheme: mgr.GetScheme(), AuthConfigClient: authConfigClient, RouteOptionClient: routeOptionClient, StatusReporter: statusReporter, @@ -897,6 +900,11 @@ func RunGlooWithExtensions(opts bootstrap.Opts, extensions Extensions) error { setupLog.Error(err, "unable to create k8s gw extensions") return err } + k8sValidator := k8svalidation.ValidationHelper{ + K8sGwExtensions: k8sGwExtensions, + GatewayQueries: query.NewData(mgr.GetClient(), mgr.GetScheme()), + Cl: mgr.GetClient(), + } // filter the list of extensions to only include the rate limit extension for validation syncerValidatorExtensions := []syncer.TranslatorSyncerExtension{} @@ -916,8 +924,7 @@ func RunGlooWithExtensions(opts bootstrap.Opts, extensions Extensions) error { GlooValidator: validator.ValidateGloo, ExtensionValidator: extensionValidator, DisableValidationAgainstPreviousState: disableValidationAgainstPreviousState, - Mgr: mgr, - K8sGwExtensions: k8sGwExtensions, + K8sGatewayValidator: k8sValidator, } if gwOpts.Validation != nil { valOpts := gwOpts.Validation From fb683d0417487028cfa881654f4d088f7e6b2964 Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Fri, 3 May 2024 15:39:19 -0500 Subject: [PATCH 05/30] hermetic dummy proxy validation --- projects/gateway/pkg/validation/validator.go | 14 +- projects/gateway2/validation/validator.go | 75 ++++++++- .../validation/validator_suite_test.go | 3 + .../gateway2/validation/validator_test.go | 143 +++++++++++++++++- 4 files changed, 221 insertions(+), 14 deletions(-) diff --git a/projects/gateway/pkg/validation/validator.go b/projects/gateway/pkg/validation/validator.go index 3439fb5cce6..f63e2fe48e7 100644 --- a/projects/gateway/pkg/validation/validator.go +++ b/projects/gateway/pkg/validation/validator.go @@ -302,15 +302,17 @@ func isK8sGatewayProxy(res resources.Resource) bool { func (v *validator) translateK8sGatewayProxies( ctx context.Context, + snap *gloov1snap.ApiSnapshot, res resources.Resource, ) ([]*gloov1.Proxy, error) { - return v.k8sGatewayValidator.TranslateK8sGatewayProxies(ctx, res) + return v.k8sGatewayValidator.TranslateK8sGatewayProxies(ctx, snap, res) } -// validateProxiesAndExtensions validates a snapshot against the Gloo and Gateway Translations. This was removed from the -// main validation loop to allow it to be re-run against the original snapshot. The reason for this second validaiton run is to allow -// the deletion of secrets, but only if they are not in use by the snapshot. This function does not know about -// those use cases, but supports them with the opts.collectAllErrors flag, which is passed as 'true' when +// validateProxiesAndExtensions validates a snapshot against the Gloo and Gateway Translations. The supplied snapshot should have either been +// modified to contain the resource being validated or in the case of DELETE validation, have the resource in question removed. +// This was removed from the main validation loop to allow it to be re-run against the original snapshot. +// The reason for this second validaiton run is to allow the deletion of secrets, but only if they are not in use by the snapshot. +// This function does not know about those use cases, but supports them with the opts.collectAllErrors flag, which is passed as 'true' when // attempting to delete a secret. This flag overrides the usual behavior of continuing to the next proxy after the first error, // and instead collects all errors. // @@ -346,7 +348,7 @@ func (v *validator) validateProxiesAndExtensions(ctx context.Context, snapshot * ) if isK8sGatewayProxy(opts.Resource) { - proxies, errs = v.translateK8sGatewayProxies(ctx, opts.Resource) + proxies, errs = v.translateK8sGatewayProxies(ctx, snapshot, opts.Resource) } else { proxies, errs = v.translateGlooEdgeProxies(ctx, snapshot, opts.collectAllErrors) } diff --git a/projects/gateway2/validation/validator.go b/projects/gateway2/validation/validator.go index 7af7cf2f31e..e50a5825f22 100644 --- a/projects/gateway2/validation/validator.go +++ b/projects/gateway2/validation/validator.go @@ -9,7 +9,10 @@ import ( "github.com/solo-io/gloo/projects/gateway2/reports" "github.com/solo-io/gloo/projects/gateway2/translator" gloov1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" + "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/gloosnapshot" + "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/static" "github.com/solo-io/solo-kit/pkg/api/v1/resources" + "github.com/solo-io/solo-kit/pkg/api/v1/resources/core" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" gwv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -22,14 +25,76 @@ type ValidationHelper struct { Cl client.Client } -type ValidatorClient struct { -} +func (v *ValidationHelper) TranslateK8sGatewayProxies(ctx context.Context, snapshot *gloosnapshot.ApiSnapshot, res resources.Resource) ([]*gloov1.Proxy, error) { + us := gloov1.NewUpstream("default", "zzz_fake-upstream-for-gloo-validation") + us.UpstreamType = &gloov1.Upstream_Static{ + Static: &static.UpstreamSpec{ + Hosts: []*static.Host{ + {Addr: "solo.io", Port: 80}, + }, + }, + } + snapshot.UpsertToResourceList(us) + + routes := []*gloov1.Route{{ + Action: &gloov1.Route_RouteAction{ + RouteAction: &gloov1.RouteAction{ + Destination: &gloov1.RouteAction_Single{ + Single: &gloov1.Destination{ + DestinationType: &gloov1.Destination_Upstream{ + Upstream: us.GetMetadata().Ref(), + }, + }, + }, + }, + }, + }} + + aggregateListener := &gloov1.Listener{ + Name: "aggregate-listener", + BindAddress: "127.0.0.1", + BindPort: 8082, + ListenerType: &gloov1.Listener_AggregateListener{ + AggregateListener: &gloov1.AggregateListener{ + HttpResources: &gloov1.AggregateListener_HttpResources{ + VirtualHosts: map[string]*gloov1.VirtualHost{ + "virt1": { + Name: "virt1", + Domains: []string{"*"}, + Routes: routes, + }, + }, + }, + HttpFilterChains: []*gloov1.AggregateListener_HttpFilterChain{{ + HttpOptionsRef: "opts1", + VirtualHostRefs: []string{"virt1"}, + }}, + }, + }, + } + + proxy := &gloov1.Proxy{ + Metadata: &core.Metadata{ + Name: "zzz-fake-proxy-for-validation", + Namespace: "gloo-system", + }, + Listeners: []*gloov1.Listener{ + aggregateListener, + }, + } + + // add the policy object we are validating to the correct location + switch policy := res.(type) { + case *sologatewayv1.RouteOption: + routes[0].Options = policy.GetOptions() + case *sologatewayv1.VirtualHostOption: + aggregateListener.GetAggregateListener().GetHttpResources().VirtualHosts["virt1"].Options = policy.GetOptions() + } -func (v *ValidatorClient) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { - return nil + return []*gloov1.Proxy{proxy}, nil } -func (v *ValidationHelper) TranslateK8sGatewayProxies(ctx context.Context, res resources.Resource) ([]*gloov1.Proxy, error) { +func (v *ValidationHelper) TranslateK8sGatewayProxiesFull(ctx context.Context, res resources.Resource) ([]*gloov1.Proxy, error) { // we need to find the Gateway associated with the resource rtOpt, ok := res.(*sologatewayv1.RouteOption) if !ok { diff --git a/projects/gateway2/validation/validator_suite_test.go b/projects/gateway2/validation/validator_suite_test.go index 61168204bed..9200d664ebe 100644 --- a/projects/gateway2/validation/validator_suite_test.go +++ b/projects/gateway2/validation/validator_suite_test.go @@ -7,7 +7,10 @@ import ( . "github.com/onsi/gomega" ) +var T *testing.T + func TestValidator(t *testing.T) { + T = t RegisterFailHandler(Fail) RunSpecs(t, "Validator Suite") } diff --git a/projects/gateway2/validation/validator_test.go b/projects/gateway2/validation/validator_test.go index 6ffa6c2dd4f..20b101bf39d 100644 --- a/projects/gateway2/validation/validator_test.go +++ b/projects/gateway2/validation/validator_test.go @@ -3,12 +3,14 @@ package validation_test import ( "context" + "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo/v2" "google.golang.org/protobuf/types/known/wrapperspb" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" "sigs.k8s.io/controller-runtime/pkg/client" - // . "github.com/onsi/gomega" + . "github.com/onsi/gomega" "github.com/solo-io/gloo/pkg/utils/statusutils" sologatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" @@ -21,15 +23,28 @@ import ( "github.com/solo-io/gloo/projects/gateway2/translator/testutils" "github.com/solo-io/gloo/projects/gateway2/validation" "github.com/solo-io/gloo/projects/gateway2/wellknown" + envoybuffer "github.com/solo-io/gloo/projects/gloo/pkg/api/external/envoy/extensions/filters/http/buffer/v3" v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" extauth "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/enterprise/options/extauth/v1" "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/faultinjection" + "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/registry" + "github.com/solo-io/gloo/projects/gloo/pkg/syncer/sanitizer" + "github.com/solo-io/gloo/projects/gloo/pkg/translator" + mock_consul "github.com/solo-io/gloo/projects/gloo/pkg/upstreams/consul/mocks" + "github.com/solo-io/gloo/projects/gloo/pkg/utils" + validationutils "github.com/solo-io/gloo/projects/gloo/pkg/utils/validation" + gloovalidation "github.com/solo-io/gloo/projects/gloo/pkg/validation" + "github.com/solo-io/gloo/test/samples" corev1 "github.com/solo-io/skv2/pkg/api/core.skv2.solo.io/v1" "github.com/solo-io/solo-kit/pkg/api/v1/clients" "github.com/solo-io/solo-kit/pkg/api/v1/clients/factory" + corecache "github.com/solo-io/solo-kit/pkg/api/v1/clients/kube/cache" "github.com/solo-io/solo-kit/pkg/api/v1/clients/memory" "github.com/solo-io/solo-kit/pkg/api/v1/resources/core" "github.com/solo-io/solo-kit/pkg/api/v2/reporter" + "github.com/solo-io/solo-kit/test/matchers" k8scorev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -45,6 +60,10 @@ var _ = Describe("RouteOptionsPlugin", func() { routeOptionClient sologatewayv1.RouteOptionClient statusReporter reporter.StatusReporter inputChannels *proxy_syncer.GatewayInputChannels + + ctrl *gomock.Controller + settings *v1.Settings + registeredPlugins []plugins.Plugin ) BeforeEach(func() { @@ -62,13 +81,53 @@ var _ = Describe("RouteOptionsPlugin", func() { statusReporter = reporter.NewReporter("gloo-kube-gateway", statusClient, routeOptionClient.BaseClient()) inputChannels = proxy_syncer.NewGatewayInputChannels() + + ctrl = gomock.NewController(T) + kube := fake.NewSimpleClientset() + kubeCoreCache, err := corecache.NewKubeCoreCache(context.Background(), kube) + Expect(err).NotTo(HaveOccurred()) + + opts := bootstrap.Opts{ + Settings: settings, + Secrets: resourceClientFactory, + Upstreams: resourceClientFactory, + Consul: bootstrap.Consul{ + ConsulWatcher: mock_consul.NewMockConsulWatcher(ctrl), // just needed to activate the consul plugin + }, + KubeClient: kube, + KubeCoreCache: kubeCoreCache, + } + registeredPlugins = registry.Plugins(opts) }) AfterEach(func() { cancel() }) - It("validates a RouteOption", func() { + FIt("validates a RouteOption with a dummy proxy", func() { + routeReplacingSanitizer, _ := sanitizer.NewRouteReplacingSanitizer(settings.GetGloo().GetInvalidConfigPolicy()) + xdsSanitizer := sanitizer.XdsSanitizers{ + sanitizer.NewUpstreamRemovingSanitizer(), + routeReplacingSanitizer, + } + + pluginRegistry := registry.NewPluginRegistry(registeredPlugins) + + translator := translator.NewTranslatorWithHasher( + utils.NewSslConfigTranslator(), + settings, + pluginRegistry, + translator.EnvoyCacheResourcesListToFnvHash, + ) + vc := gloovalidation.ValidatorConfig{ + Ctx: context.Background(), + GlooValidatorConfig: gloovalidation.GlooValidatorConfig{ + XdsSanitizer: xdsSanitizer, + Translator: translator, + }, + } + gv := gloovalidation.NewValidator(vc) + deps := []client.Object{svc(), gw(), httpRoute(), attachedRouteOption()} routeOptionClient.Write(attachedInternal(), clients.WriteOpts{}) fakeClient := testutils.BuildIndexedFakeClient(deps, gwquery.IterateIndices, rtoptquery.IterateIndices) @@ -89,8 +148,56 @@ var _ = Describe("RouteOptionsPlugin", func() { GatewayQueries: gwQueries, Cl: fakeClient, } - validator.TranslateK8sGatewayProxies(ctx, rtOpt) + + params := plugins.Params{ + Ctx: context.Background(), + Snapshot: samples.SimpleGlooSnapshot("gloo-system"), + } + proxies, _ := validator.TranslateK8sGatewayProxies(ctx, params.Snapshot, rtOpt) + params.Snapshot.Proxies = proxies + gv.Sync(ctx, params.Snapshot) + rpt, err := gv.ValidateGloo(ctx, proxies[0], rtOpt, false) + Expect(err).NotTo(HaveOccurred()) + r := rpt[0] + Expect(r.Proxy).To(Equal(proxies[0])) + Expect(r.ResourceReports).To(Equal(reporter.ResourceReports{})) + Expect(r.ProxyReport).To(matchers.MatchProto(validationutils.MakeReport(proxies[0]))) + + vhost := attachedVHostInternal() + proxies, _ = validator.TranslateK8sGatewayProxies(ctx, params.Snapshot, vhost) + params.Snapshot.Proxies = proxies + gv.Sync(ctx, params.Snapshot) + rpt, err = gv.ValidateGloo(ctx, proxies[0], vhost, false) + Expect(err).NotTo(HaveOccurred()) + r = rpt[0] + Expect(r.Proxy).To(Equal(proxies[0])) + Expect(r.ResourceReports).To(Equal(reporter.ResourceReports{})) + Expect(r.ProxyReport).To(matchers.MatchProto(validationutils.MakeReport(proxies[0]))) }) + + // It("validates a RouteOption", func() { + // deps := []client.Object{svc(), gw(), httpRoute(), attachedRouteOption()} + // routeOptionClient.Write(attachedInternal(), clients.WriteOpts{}) + // fakeClient := testutils.BuildIndexedFakeClient(deps, gwquery.IterateIndices, rtoptquery.IterateIndices) + // gwQueries := testutils.BuildGatewayQueriesWithClient(fakeClient) + + // k8sGwExtensions, _ := extensions.NewK8sGatewayExtensions(ctx, extensions.K8sGatewayExtensionsFactoryParameters{ + // Cl: fakeClient, + // Scheme: sch, + // AuthConfigClient: authConfigClient, + // RouteOptionClient: routeOptionClient, + // StatusReporter: statusReporter, + // KickXds: inputChannels.Kick, + // }) + + // rtOpt := attachedInternal() + // validator := validation.ValidationHelper{ + // K8sGwExtensions: k8sGwExtensions, + // GatewayQueries: gwQueries, + // Cl: fakeClient, + // } + // validator.TranslateK8sGatewayProxies(ctx, rtOpt) + // }) }) func nsPtr(s string) *gwv1.Namespace { @@ -178,6 +285,36 @@ func gw() *gwv1.Gateway { } } +func attachedVirtualHostOption() *solokubev1.VirtualHostOption { + return &solokubev1.VirtualHostOption{ + ObjectMeta: metav1.ObjectMeta{ + Name: "policy", + Namespace: "default", + }, + Spec: *attachedVHostInternal(), + } +} + +func attachedVHostInternal() *sologatewayv1.VirtualHostOption { + return &sologatewayv1.VirtualHostOption{ + TargetRef: &corev1.PolicyTargetReferenceWithSectionName{ + Group: gwv1.GroupVersion.Group, + Kind: wellknown.GatewayKind, + Name: "gw", + Namespace: wrapperspb.String("default"), + }, + Options: &v1.VirtualHostOptions{ + BufferPerRoute: &envoybuffer.BufferPerRoute{ + Override: &envoybuffer.BufferPerRoute_Buffer{ + Buffer: &envoybuffer.Buffer{ + MaxRequestBytes: nil, + }, + }, + }, + }, + } +} + func attachedRouteOption() *solokubev1.RouteOption { now := metav1.Now() return &solokubev1.RouteOption{ From 91edb5913d04901ba66223f2c7645e334e860a9a Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Mon, 6 May 2024 21:31:05 -0500 Subject: [PATCH 06/30] use simple ggv2 validation helper for hermetic validation --- ...eway-validation-webhook-configuration.yaml | 4 + .../validating_admission_webhook_test.go | 48 +++- projects/gateway/pkg/validation/validator.go | 38 +-- .../gateway/pkg/validation/validator_test.go | 21 -- .../controller/controller_suite_test.go | 3 +- projects/gateway2/controller/start.go | 75 ++++-- ...mple-http-route-with-attached-options.yaml | 2 +- projects/gateway2/extensions/extensions.go | 37 ++- .../testutils/validation_test_utils.go | 64 ----- projects/gateway2/validation/validator.go | 88 ++---- .../gateway2/validation/validator_test.go | 252 +++--------------- .../gloo/pkg/syncer/setup/setup_syncer.go | 69 +---- projects/gloo/pkg/syncer/setup/start_func.go | 26 +- 13 files changed, 233 insertions(+), 494 deletions(-) diff --git a/install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml b/install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml index ca02ffc6a84..c966de36a99 100644 --- a/install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml +++ b/install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml @@ -24,6 +24,10 @@ webhooks: path: "/validation" caBundle: "" # update manually or use certgen job or cert-manager's ca-injector rules: + - operations: [ "CREATE", "UPDATE" ] + apiGroups: ["gateway.solo.io"] + apiVersions: ["v1"] + resources: ["routeoptions", "virtualhostoptions"] - operations: {{ include "gloo.webhookvalidation.operationsForResource" (list "virtualservices" .Values.gateway.validation.webhook.skipDeleteValidationResources) }} apiGroups: ["gateway.solo.io"] apiVersions: ["v1"] diff --git a/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook_test.go b/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook_test.go index 1f1771e0d72..cb804529b4c 100644 --- a/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook_test.go +++ b/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook_test.go @@ -16,6 +16,8 @@ import ( "github.com/rotisserie/eris" "github.com/solo-io/gloo/projects/gateway/pkg/validation" validation2 "github.com/solo-io/gloo/projects/gloo/pkg/api/grpc/validation" + "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/faultinjection" + "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/headers" "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/static" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" @@ -37,11 +39,11 @@ import ( var _ = Describe("ValidatingAdmissionWebhook", func() { + const errMsg = "didn't say the magic word" var ( - srv *httptest.Server - mv *mockValidator - wh *gatewayValidationWebhook - errMsg = "didn't say the magic word" + srv *httptest.Server + mv *mockValidator + wh *gatewayValidationWebhook ) BeforeEach(func() { @@ -110,6 +112,32 @@ var _ = Describe("ValidatingAdmissionWebhook", func() { routeTable := &v1.RouteTable{Metadata: &core.Metadata{Namespace: "namespace", Name: "rt"}} + routeOption := &v1.RouteOption{ + Metadata: &core.Metadata{ + Name: "policy", + Namespace: "default", + }, + Options: &gloov1.RouteOptions{ + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + Percentage: 4.19, + HttpStatus: 500, + }, + }, + }, + } + vHostOption := &v1.VirtualHostOption{ + Metadata: &core.Metadata{ + Name: "policy", + Namespace: "default", + }, + Options: &gloov1.VirtualHostOptions{ + HeaderManipulation: &headers.HeaderManipulation{ + RequestHeadersToRemove: []string{"hello"}, + }, + }, + } + DescribeTable("processes admission requests with auto-accept validator", func(crd crd.Crd, gvk schema.GroupVersionKind, op v1beta1.Operation, resourceOrRef interface{}) { reviewRequest := makeReviewRequest(srv.URL, crd, gvk, op, resourceOrRef) res, err := srv.Client().Do(reviewRequest) @@ -131,6 +159,8 @@ var _ = Describe("ValidatingAdmissionWebhook", func() { Entry("upstream, accepted", gloov1.UpstreamCrd, gloov1.UpstreamCrd.GroupVersionKind(), v1beta1.Create, upstream), Entry("upstream deletion, accepted", gloov1.UpstreamCrd, gloov1.UpstreamCrd.GroupVersionKind(), v1beta1.Delete, upstream.GetMetadata().Ref()), Entry("secret deletion, accepted", gloov1.SecretCrd, gloov1.SecretCrd.GroupVersionKind(), v1beta1.Delete, secret.GetMetadata().Ref()), + Entry("route option, accepted", v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), + Entry("virtualHost option, accepted", v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), ) DescribeTable("processes admission requests with auto-fail validator", func(crd crd.Crd, gvk schema.GroupVersionKind, op v1beta1.Operation, resourceOrRef interface{}) { @@ -159,6 +189,8 @@ var _ = Describe("ValidatingAdmissionWebhook", func() { Entry("upstream, rejected", gloov1.UpstreamCrd, gloov1.UpstreamCrd.GroupVersionKind(), v1beta1.Create, upstream), Entry("upstream deletion, rejected", gloov1.UpstreamCrd, gloov1.UpstreamCrd.GroupVersionKind(), v1beta1.Delete, upstream.GetMetadata().Ref()), Entry("secret deletion, rejected", gloov1.SecretCrd, gloov1.SecretCrd.GroupVersionKind(), v1beta1.Delete, secret.GetMetadata().Ref()), + Entry("route option, rejected", v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), + Entry("virtualHost option, rejected", v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), ) DescribeTable("processes status updates with auto-fail validator", func(expectAllowed bool, crd crd.Crd, gvk schema.GroupVersionKind, op v1beta1.Operation, resource resources.InputResource) { @@ -238,6 +270,10 @@ var _ = Describe("ValidatingAdmissionWebhook", func() { Entry("route table update, accepted", true, v1.RouteTableCrd, v1.RouteTableCrd.GroupVersionKind(), v1beta1.Update, routeTable), Entry("upstream create, rejected", false, gloov1.UpstreamCrd, gloov1.UpstreamCrd.GroupVersionKind(), v1beta1.Create, upstream), Entry("upstream update, accepted", true, gloov1.UpstreamCrd, gloov1.UpstreamCrd.GroupVersionKind(), v1beta1.Update, upstream), + Entry("route option create, rejected", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), + Entry("route option update, accepted", true, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Update, routeOption), + Entry("virtualHost create, rejected", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), + Entry("virtualHost update, accepted", true, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Update, vHostOption), ) DescribeTable("processes metadata updates with auto-fail validator", func(expectAllowed bool, crd crd.Crd, gvk schema.GroupVersionKind, op v1beta1.Operation, resource resources.InputResource) { @@ -307,6 +343,10 @@ var _ = Describe("ValidatingAdmissionWebhook", func() { Entry("route table update, rejected", false, v1.RouteTableCrd, v1.RouteTableCrd.GroupVersionKind(), v1beta1.Update, routeTable), Entry("upstream create, rejected", false, gloov1.UpstreamCrd, gloov1.UpstreamCrd.GroupVersionKind(), v1beta1.Create, upstream), Entry("upstream update, rejected", false, gloov1.UpstreamCrd, gloov1.UpstreamCrd.GroupVersionKind(), v1beta1.Update, upstream), + Entry("route option create, rejected", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), + Entry("route option update, rejected", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Update, routeOption), + Entry("virtualHost create, rejected", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), + Entry("virtualHost update, rejected", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Update, vHostOption), ) Context("invalid yaml", func() { diff --git a/projects/gateway/pkg/validation/validator.go b/projects/gateway/pkg/validation/validator.go index f63e2fe48e7..b85f23eb35f 100644 --- a/projects/gateway/pkg/validation/validator.go +++ b/projects/gateway/pkg/validation/validator.go @@ -16,7 +16,7 @@ import ( sologatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" "github.com/solo-io/gloo/projects/gateway/pkg/translator" "github.com/solo-io/gloo/projects/gateway/pkg/utils" - k8svalidation "github.com/solo-io/gloo/projects/gateway2/validation" + k8sgwvalidation "github.com/solo-io/gloo/projects/gateway2/validation" "github.com/solo-io/gloo/projects/gloo/pkg/api/grpc/validation" gloov1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" syncerValidation "github.com/solo-io/gloo/projects/gloo/pkg/syncer/validation" @@ -122,7 +122,6 @@ type validator struct { extensionValidator syncerValidation.Validator allowWarnings bool disableValidationAgainstPreviousState bool - k8sGatewayValidator k8svalidation.ValidationHelper } type validationOptions struct { @@ -145,7 +144,6 @@ type ValidatorConfig struct { ExtensionValidator syncerValidation.Validator AllowWarnings bool DisableValidationAgainstPreviousState bool - K8sGatewayValidator k8svalidation.ValidationHelper } func NewValidator(cfg ValidatorConfig) *validator { @@ -155,7 +153,6 @@ func NewValidator(cfg ValidatorConfig) *validator { translator: cfg.Translator, allowWarnings: cfg.AllowWarnings, disableValidationAgainstPreviousState: cfg.DisableValidationAgainstPreviousState, - k8sGatewayValidator: cfg.K8sGatewayValidator, } } @@ -293,19 +290,16 @@ func (v *validator) translateGlooEdgeProxies( return proxies, errs } +// isK8sGatewayProxy returns true if we are evaluating a Policy resource in the context of K8s Gateway API support. +// Currently the only signal needed to make this decision is if the resource being evaluated is a RouteOption +// or a VirthalHostOption, as we only directly evaluate them in the validator to support K8s Gateway API mode. func isK8sGatewayProxy(res resources.Resource) bool { - if _, ok := res.(*sologatewayv1.RouteOption); ok { + switch res.(type) { + case *sologatewayv1.RouteOption, *sologatewayv1.VirtualHostOption: return true + default: + return false } - return false -} - -func (v *validator) translateK8sGatewayProxies( - ctx context.Context, - snap *gloov1snap.ApiSnapshot, - res resources.Resource, -) ([]*gloov1.Proxy, error) { - return v.k8sGatewayValidator.TranslateK8sGatewayProxies(ctx, snap, res) } // validateProxiesAndExtensions validates a snapshot against the Gloo and Gateway Translations. The supplied snapshot should have either been @@ -347,8 +341,9 @@ func (v *validator) validateProxiesAndExtensions(ctx context.Context, snapshot * err error ) - if isK8sGatewayProxy(opts.Resource) { - proxies, errs = v.translateK8sGatewayProxies(ctx, snapshot, opts.Resource) + validatingK8sGateway := isK8sGatewayProxy(opts.Resource) + if validatingK8sGateway { + proxies, errs = k8sgwvalidation.TranslateK8sGatewayProxies(ctx, snapshot, opts.Resource) } else { proxies, errs = v.translateGlooEdgeProxies(ctx, snapshot, opts.collectAllErrors) } @@ -381,6 +376,16 @@ func (v *validator) validateProxiesAndExtensions(ctx context.Context, snapshot * continue } + // if we are validating K8s Gateway Policy resources, we only want the errors resulting from + // the RouteOption/VirtualHostOption, so let's grab that here and skip the more sophisticated + // error aggregation below + if validatingK8sGateway { + if err = k8sgwvalidation.GetSimpleErrorFromGlooValidation(glooReports, proxy, true); err != nil { + errs = multierror.Append(errs, err) + } + continue + } + // Collect the reports returned by the glooValidator proxyReport := glooReports[0].ProxyReport proxyReports = append(proxyReports, proxyReport) @@ -399,7 +404,6 @@ func (v *validator) validateProxiesAndExtensions(ctx context.Context, snapshot * err = errors.Wrapf(err, failedResourceReports) errs = multierror.Append(errs, err) } - } // End of proxy validation loop // Extension validation. Currently only supports rate limit. diff --git a/projects/gateway/pkg/validation/validator_test.go b/projects/gateway/pkg/validation/validator_test.go index 68a483ceff0..b0213d568af 100644 --- a/projects/gateway/pkg/validation/validator_test.go +++ b/projects/gateway/pkg/validation/validator_test.go @@ -14,7 +14,6 @@ import ( "github.com/solo-io/gloo/pkg/utils" v1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" "github.com/solo-io/gloo/projects/gateway/pkg/translator" - validationtestutils "github.com/solo-io/gloo/projects/gateway2/validation/testutils" "github.com/solo-io/gloo/projects/gloo/pkg/api/grpc/validation" gloov1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" gloov1snap "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/gloosnapshot" @@ -462,26 +461,6 @@ var _ = Describe("Validator", func() { }) - Context("validating route option in ggv2", func() { - Context("proxy validation accepted", func() { - FIt("accepts the rtopt", func() { - ctx := context.Background() - - v.k8sGatewayValidator = validationtestutils.BuildValidationHelper() - - v.glooValidator = ValidateAccept - // simple snapshot is fine, for kube gateway validation we don't need the RouteOption to be part of the snap - snap := samples.SimpleGlooSnapshot(ns) - err := v.Sync(context.TODO(), snap) - Expect(err).NotTo(HaveOccurred()) - - reports, err := v.ValidateModifiedGvk(ctx, v1.RouteOptionGVK, validationtestutils.AttachedInternal(), false) - Expect(err).NotTo(HaveOccurred()) - Expect(*(reports.ProxyReports)).To(HaveLen(1)) - }) - }) - }) - Context("validating a route table", func() { Context("proxy validation accepted", func() { It("accepts the rt", func() { diff --git a/projects/gateway2/controller/controller_suite_test.go b/projects/gateway2/controller/controller_suite_test.go index 1d8f9350851..fb2f39ad57e 100644 --- a/projects/gateway2/controller/controller_suite_test.go +++ b/projects/gateway2/controller/controller_suite_test.go @@ -99,8 +99,7 @@ var _ = BeforeSuite(func() { var gatewayClassObjName api.ObjectName = api.ObjectName(gatewayClassName) exts, err := extensions.NewK8sGatewayExtensions(ctx, extensions.K8sGatewayExtensionsFactoryParameters{ - Cl: k8sClient, - Scheme: scheme, + Mgr: mgr, }) Expect(err).ToNot(HaveOccurred()) cfg := controller.GatewayConfig{ diff --git a/projects/gateway2/controller/start.go b/projects/gateway2/controller/start.go index 9816bef595e..a5662477187 100644 --- a/projects/gateway2/controller/start.go +++ b/projects/gateway2/controller/start.go @@ -5,9 +5,12 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/healthz" - "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" apiv1 "sigs.k8s.io/gateway-api/apis/v1" + gatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" + "github.com/solo-io/gloo/projects/gateway2/controller/scheme" "github.com/solo-io/gloo/projects/gateway2/extensions" "github.com/solo-io/gloo/projects/gateway2/proxy_syncer" "github.com/solo-io/gloo/projects/gateway2/secrets" @@ -16,6 +19,7 @@ import ( api "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/enterprise/options/extauth/v1" "github.com/solo-io/gloo/projects/gloo/pkg/bootstrap" "github.com/solo-io/gloo/projects/gloo/pkg/plugins" + "github.com/solo-io/solo-kit/pkg/api/v2/reporter" ) const ( @@ -36,7 +40,7 @@ type StartConfig struct { // ExtensionsFactory is the factory function which will return an extensions.K8sGatewayExtensions // This is responsible for producing the extension points that this controller requires - K8sGatewayExtensions extensions.K8sGatewayExtensions + ExtensionsFactory extensions.K8sGatewayExtensionsFactory // GlooPluginRegistryFactory is the factory function to produce a PluginRegistry // The plugins in this registry are used during the conversion of a Proxy resource into an xDS Snapshot @@ -46,12 +50,18 @@ type StartConfig struct { // This cache is utilized by the debug.ProxyEndpointServer ProxyClient v1.ProxyClient - InputChannels *proxy_syncer.GatewayInputChannels - - Mgr manager.Manager // AuthConfigClient is the client used for retrieving AuthConfig objects within the Portal Plugin AuthConfigClient api.AuthConfigClient + // RouteOptionClient is the client used for retrieving RouteOption objects within the RouteOptionsPlugin + // NOTE: We may be able to move this entirely to the RouteOptionsPlugin + RouteOptionClient gatewayv1.RouteOptionClient + // VirtualHostOptionClient is the client used for retrieving VirtualHostOption objects within the VirtualHostOptionsPlugin + // NOTE: We may be able to move this entirely to the VirtualHostOptionsPlugin + VirtualHostOptionClient gatewayv1.VirtualHostOptionClient + // StatusReporter is used within any StatusPlugins that must persist a GE-classic style status + StatusReporter reporter.StatusReporter + // A callback to initialize the gateway status syncer with the same dependencies // as the gateway controller (in another start func) // TODO(ilackarms) refactor to enable the status syncer to be started in the same start func @@ -62,24 +72,53 @@ type StartConfig struct { // It is intended to be run in a goroutine as the function will block until the supplied // context is cancelled func Start(ctx context.Context, cfg StartConfig) error { - // var opts []zap.Opts - // if cfg.Dev { - // setupLog.Info("starting log in dev mode") - // opts = append(opts, zap.UseDevMode(true)) - // } - // ctrl.SetLogger(zap.New(opts...)) - - mgr := cfg.Mgr + var opts []zap.Opts + if cfg.Dev { + setupLog.Info("starting log in dev mode") + opts = append(opts, zap.UseDevMode(true)) + } + ctrl.SetLogger(zap.New(opts...)) + + mgrOpts := ctrl.Options{ + Scheme: scheme.NewScheme(), + PprofBindAddress: "127.0.0.1:9099", + // if you change the port here, also change the port "health" in the helmchart. + HealthProbeBindAddress: ":9093", + Metrics: metricsserver.Options{ + BindAddress: ":9092", + }, + } + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), mgrOpts) + if err != nil { + setupLog.Error(err, "unable to start manager") + return err + } + // TODO: replace this with something that checks that we have xds snapshot ready (or that we don't need one). mgr.AddReadyzCheck("ready-ping", healthz.Ping) + inputChannels := proxy_syncer.NewGatewayInputChannels() + + k8sGwExtensions, err := cfg.ExtensionsFactory(ctx, extensions.K8sGatewayExtensionsFactoryParameters{ + Mgr: mgr, + RouteOptionClient: cfg.RouteOptionClient, + VirtualHostOptionClient: cfg.VirtualHostOptionClient, + StatusReporter: cfg.StatusReporter, + KickXds: inputChannels.Kick, + AuthConfigClient: cfg.AuthConfigClient, + }) + if err != nil { + setupLog.Error(err, "unable to create k8s gw extensions") + return err + } + // Create the proxy syncer for the Gateway API resources proxySyncer := proxy_syncer.NewProxySyncer( wellknown.GatewayControllerName, cfg.Opts.WriteNamespace, - cfg.InputChannels, + inputChannels, mgr, - cfg.K8sGatewayExtensions, + k8sGwExtensions, cfg.ProxyClient, cfg.QueueStatusForProxies, ) @@ -95,15 +134,15 @@ func Start(ctx context.Context, cfg StartConfig) error { AutoProvision: AutoProvision, ControlPlane: cfg.Opts.ControlPlane, IstioValues: cfg.Opts.GlooGateway.IstioValues, - Kick: cfg.InputChannels.Kick, - Extensions: cfg.K8sGatewayExtensions, + Kick: inputChannels.Kick, + Extensions: k8sGwExtensions, } if err := NewBaseGatewayController(ctx, gwCfg); err != nil { setupLog.Error(err, "unable to create controller") return err } - if err := secrets.NewSecretsController(ctx, mgr, cfg.InputChannels); err != nil { + if err := secrets.NewSecretsController(ctx, mgr, inputChannels); err != nil { setupLog.Error(err, "unable to create controller") return err } diff --git a/projects/gateway2/examples/example-http-route-with-attached-options.yaml b/projects/gateway2/examples/example-http-route-with-attached-options.yaml index f0dc6eb4a6e..e7bc5c552f0 100644 --- a/projects/gateway2/examples/example-http-route-with-attached-options.yaml +++ b/projects/gateway2/examples/example-http-route-with-attached-options.yaml @@ -33,7 +33,7 @@ spec: faults: abort: percentage: 100 - httpStatus: 500 + #httpStatus: 500 timeout: 11s --- apiVersion: v1 diff --git a/projects/gateway2/extensions/extensions.go b/projects/gateway2/extensions/extensions.go index 3fd5da3a6a7..272f0b38d28 100644 --- a/projects/gateway2/extensions/extensions.go +++ b/projects/gateway2/extensions/extensions.go @@ -4,14 +4,13 @@ import ( "context" v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/enterprise/options/extauth/v1" - "k8s.io/apimachinery/pkg/runtime" "github.com/solo-io/gloo/pkg/version" gatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" "github.com/solo-io/gloo/projects/gateway2/query" "github.com/solo-io/gloo/projects/gateway2/translator/plugins/registry" "github.com/solo-io/solo-kit/pkg/api/v2/reporter" - "sigs.k8s.io/controller-runtime/pkg/client" + controllerruntime "sigs.k8s.io/controller-runtime" ) // K8sGatewayExtensions is responsible for providing implementations for translation utilities @@ -26,12 +25,12 @@ type K8sGatewayExtensions interface { // K8sGatewayExtensionsFactoryParameters contains the parameters required to start Gloo K8s Gateway Extensions (including Translator Plugins) type K8sGatewayExtensionsFactoryParameters struct { - Cl client.Client - Scheme *runtime.Scheme - AuthConfigClient v1.AuthConfigClient - RouteOptionClient gatewayv1.RouteOptionClient - StatusReporter reporter.StatusReporter - KickXds func(ctx context.Context) + Mgr controllerruntime.Manager + AuthConfigClient v1.AuthConfigClient + RouteOptionClient gatewayv1.RouteOptionClient + VirtualHostOptionClient gatewayv1.VirtualHostOptionClient + StatusReporter reporter.StatusReporter + KickXds func(ctx context.Context) } // K8sGatewayExtensionsFactory returns an extensions.K8sGatewayExtensions @@ -46,29 +45,29 @@ func NewK8sGatewayExtensions( params K8sGatewayExtensionsFactoryParameters, ) (K8sGatewayExtensions, error) { return &k8sGatewayExtensions{ - params.Cl, - params.Scheme, - params.RouteOptionClient, - params.StatusReporter, + mgr: params.Mgr, + routeOptionClient: params.RouteOptionClient, + virtualHostOptionClient: params.VirtualHostOptionClient, + statusReporter: params.StatusReporter, }, nil } type k8sGatewayExtensions struct { - cl client.Client - scheme *runtime.Scheme - routeOptionClient gatewayv1.RouteOptionClient - statusReporter reporter.StatusReporter + mgr controllerruntime.Manager + routeOptionClient gatewayv1.RouteOptionClient + virtualHostOptionClient gatewayv1.VirtualHostOptionClient + statusReporter reporter.StatusReporter } // CreatePluginRegistry returns the PluginRegistry func (e *k8sGatewayExtensions) CreatePluginRegistry(_ context.Context) registry.PluginRegistry { queries := query.NewData( - e.cl, - e.scheme, + e.mgr.GetClient(), + e.mgr.GetScheme(), ) plugins := registry.BuildPlugins( queries, - e.cl, + e.mgr.GetClient(), e.routeOptionClient, e.statusReporter, ) diff --git a/projects/gateway2/validation/testutils/validation_test_utils.go b/projects/gateway2/validation/testutils/validation_test_utils.go index 6bd5deb2616..5c7248658bb 100644 --- a/projects/gateway2/validation/testutils/validation_test_utils.go +++ b/projects/gateway2/validation/testutils/validation_test_utils.go @@ -1,85 +1,21 @@ package testutils import ( - "context" - "google.golang.org/protobuf/types/known/wrapperspb" - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/solo-io/gloo/pkg/utils/statusutils" sologatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" solokubev1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1/kube/apis/gateway.solo.io/v1" - "github.com/solo-io/gloo/projects/gateway2/controller/scheme" - "github.com/solo-io/gloo/projects/gateway2/extensions" - "github.com/solo-io/gloo/projects/gateway2/proxy_syncer" - gwquery "github.com/solo-io/gloo/projects/gateway2/query" - rtoptquery "github.com/solo-io/gloo/projects/gateway2/translator/plugins/routeoptions/query" - "github.com/solo-io/gloo/projects/gateway2/translator/testutils" - "github.com/solo-io/gloo/projects/gateway2/validation" "github.com/solo-io/gloo/projects/gateway2/wellknown" v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" - extauth "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/enterprise/options/extauth/v1" "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/faultinjection" corev1 "github.com/solo-io/skv2/pkg/api/core.skv2.solo.io/v1" - "github.com/solo-io/solo-kit/pkg/api/v1/clients" - "github.com/solo-io/solo-kit/pkg/api/v1/clients/factory" - "github.com/solo-io/solo-kit/pkg/api/v1/clients/memory" "github.com/solo-io/solo-kit/pkg/api/v1/resources/core" - "github.com/solo-io/solo-kit/pkg/api/v2/reporter" k8scorev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" gwv1 "sigs.k8s.io/gateway-api/apis/v1" ) -func BuildValidationHelper() validation.ValidationHelper { - var ( - ctx context.Context - sch *runtime.Scheme - authConfigClient extauth.AuthConfigClient - routeOptionClient sologatewayv1.RouteOptionClient - statusReporter reporter.StatusReporter - inputChannels *proxy_syncer.GatewayInputChannels - ) - - ctx = context.Background() - - sch = scheme.NewScheme() - resourceClientFactory := &factory.MemoryResourceClientFactory{ - Cache: memory.NewInMemoryResourceCache(), - } - - routeOptionClient, _ = sologatewayv1.NewRouteOptionClient(ctx, resourceClientFactory) - authConfigClient, _ = extauth.NewAuthConfigClient(ctx, resourceClientFactory) - statusClient := statusutils.GetStatusClientForNamespace("gloo-system") - - statusReporter = reporter.NewReporter("gloo-kube-gateway", statusClient, routeOptionClient.BaseClient()) - - inputChannels = proxy_syncer.NewGatewayInputChannels() - - deps := []client.Object{svc(), gw(), httpRoute(), attachedRouteOption()} - routeOptionClient.Write(AttachedInternal(), clients.WriteOpts{}) - fakeClient := testutils.BuildIndexedFakeClient(deps, gwquery.IterateIndices, rtoptquery.IterateIndices) - gwQueries := testutils.BuildGatewayQueriesWithClient(fakeClient) - - k8sGwExtensions, _ := extensions.NewK8sGatewayExtensions(ctx, extensions.K8sGatewayExtensionsFactoryParameters{ - Cl: fakeClient, - Scheme: sch, - AuthConfigClient: authConfigClient, - RouteOptionClient: routeOptionClient, - StatusReporter: statusReporter, - KickXds: inputChannels.Kick, - }) - - validator := validation.ValidationHelper{ - K8sGwExtensions: k8sGwExtensions, - GatewayQueries: gwQueries, - Cl: fakeClient, - } - return validator -} - func nsPtr(s string) *gwv1.Namespace { var ns gwv1.Namespace = gwv1.Namespace(s) return &ns diff --git a/projects/gateway2/validation/validator.go b/projects/gateway2/validation/validator.go index e50a5825f22..4aefa3697cf 100644 --- a/projects/gateway2/validation/validator.go +++ b/projects/gateway2/validation/validator.go @@ -4,28 +4,15 @@ import ( "context" sologatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" - "github.com/solo-io/gloo/projects/gateway2/extensions" - "github.com/solo-io/gloo/projects/gateway2/query" - "github.com/solo-io/gloo/projects/gateway2/reports" - "github.com/solo-io/gloo/projects/gateway2/translator" gloov1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/gloosnapshot" "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/static" + gloovalidation "github.com/solo-io/gloo/projects/gloo/pkg/validation" "github.com/solo-io/solo-kit/pkg/api/v1/resources" "github.com/solo-io/solo-kit/pkg/api/v1/resources/core" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" - gwv1 "sigs.k8s.io/gateway-api/apis/v1" ) -// contains data needed to bootstrap a GW2 translator -type ValidationHelper struct { - K8sGwExtensions extensions.K8sGatewayExtensions - GatewayQueries query.GatewayQueries - Cl client.Client -} - -func (v *ValidationHelper) TranslateK8sGatewayProxies(ctx context.Context, snapshot *gloosnapshot.ApiSnapshot, res resources.Resource) ([]*gloov1.Proxy, error) { +func TranslateK8sGatewayProxies(ctx context.Context, snap *gloosnapshot.ApiSnapshot, res resources.Resource) ([]*gloov1.Proxy, error) { us := gloov1.NewUpstream("default", "zzz_fake-upstream-for-gloo-validation") us.UpstreamType = &gloov1.Upstream_Static{ Static: &static.UpstreamSpec{ @@ -34,9 +21,10 @@ func (v *ValidationHelper) TranslateK8sGatewayProxies(ctx context.Context, snaps }, }, } - snapshot.UpsertToResourceList(us) + snap.UpsertToResourceList(us) routes := []*gloov1.Route{{ + Name: "route", Action: &gloov1.Route_RouteAction{ RouteAction: &gloov1.RouteAction{ Destination: &gloov1.RouteAction_Single{ @@ -58,8 +46,8 @@ func (v *ValidationHelper) TranslateK8sGatewayProxies(ctx context.Context, snaps AggregateListener: &gloov1.AggregateListener{ HttpResources: &gloov1.AggregateListener_HttpResources{ VirtualHosts: map[string]*gloov1.VirtualHost{ - "virt1": { - Name: "virt1", + "vhost": { + Name: "vhost", Domains: []string{"*"}, Routes: routes, }, @@ -67,7 +55,7 @@ func (v *ValidationHelper) TranslateK8sGatewayProxies(ctx context.Context, snaps }, HttpFilterChains: []*gloov1.AggregateListener_HttpFilterChain{{ HttpOptionsRef: "opts1", - VirtualHostRefs: []string{"virt1"}, + VirtualHostRefs: []string{"vhost"}, }}, }, }, @@ -88,63 +76,25 @@ func (v *ValidationHelper) TranslateK8sGatewayProxies(ctx context.Context, snaps case *sologatewayv1.RouteOption: routes[0].Options = policy.GetOptions() case *sologatewayv1.VirtualHostOption: - aggregateListener.GetAggregateListener().GetHttpResources().VirtualHosts["virt1"].Options = policy.GetOptions() + aggregateListener.GetAggregateListener().GetHttpResources().VirtualHosts["vhost"].Options = policy.GetOptions() } return []*gloov1.Proxy{proxy}, nil } -func (v *ValidationHelper) TranslateK8sGatewayProxiesFull(ctx context.Context, res resources.Resource) ([]*gloov1.Proxy, error) { - // we need to find the Gateway associated with the resource - rtOpt, ok := res.(*sologatewayv1.RouteOption) - if !ok { - panic("uh oh") - } - - // first find the target HTTPRoute - var hr gwv1.HTTPRoute - targetRef := rtOpt.GetTargetRef() - hrnn := types.NamespacedName{ - Namespace: targetRef.GetNamespace().GetValue(), - Name: targetRef.GetName(), - } - err := v.Cl.Get(ctx, hrnn, &hr, &client.GetOptions{}) - if err != nil { - panic("err getting") - } - - // now we directly get the parentRefs (which are Gateways!) - var gws []gwv1.Gateway - for _, pr := range hr.Spec.ParentRefs { - var gw gwv1.Gateway - gwnn := types.NamespacedName{ - Namespace: string(*pr.Namespace), - Name: string(pr.Name), - } - err := v.Cl.Get(ctx, gwnn, &gw, &client.GetOptions{}) - if err != nil { - panic("err getting GW") - } - gws = append(gws, gw) - } - - // create the plugins and translator - plugins := v.K8sGwExtensions.CreatePluginRegistry(ctx) - t := translator.NewTranslator(v.GatewayQueries, plugins) - rm := reports.NewReportMap() - r := reports.NewReporter(&rm) +func GetSimpleErrorFromGlooValidation( + reports []*gloovalidation.GlooValidationReport, + proxy *gloov1.Proxy, + allowWarnings bool, +) error { + var errs error - // translate all the gateways and collect the output Proxies - var proxies []*gloov1.Proxy - for _, gw := range gws { - proxy := t.TranslateProxy(ctx, &gw, "gloo-system", r) - if proxy == nil || len(proxy.GetListeners()) == 0 { - continue + for _, report := range reports { + proxyResReport := report.ResourceReports[proxy] + if proxyResReport.Errors != nil { + return proxyResReport.Errors } - proxies = append(proxies, proxy) } - // we currently don't want to fail validation for any K8s Gateway-specific error conditions, as the behavior and status reporting - // requirements for these scenarios are built into the spec; let's always return a nil error until that decision changes - return proxies, nil + return errs } diff --git a/projects/gateway2/validation/validator_test.go b/projects/gateway2/validation/validator_test.go index 20b101bf39d..087e0b29ca3 100644 --- a/projects/gateway2/validation/validator_test.go +++ b/projects/gateway2/validation/validator_test.go @@ -6,26 +6,15 @@ import ( "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo/v2" "google.golang.org/protobuf/types/known/wrapperspb" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" - "sigs.k8s.io/controller-runtime/pkg/client" . "github.com/onsi/gomega" - "github.com/solo-io/gloo/pkg/utils/statusutils" sologatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" - solokubev1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1/kube/apis/gateway.solo.io/v1" - "github.com/solo-io/gloo/projects/gateway2/controller/scheme" - "github.com/solo-io/gloo/projects/gateway2/extensions" - "github.com/solo-io/gloo/projects/gateway2/proxy_syncer" - gwquery "github.com/solo-io/gloo/projects/gateway2/query" - rtoptquery "github.com/solo-io/gloo/projects/gateway2/translator/plugins/routeoptions/query" - "github.com/solo-io/gloo/projects/gateway2/translator/testutils" "github.com/solo-io/gloo/projects/gateway2/validation" "github.com/solo-io/gloo/projects/gateway2/wellknown" envoybuffer "github.com/solo-io/gloo/projects/gloo/pkg/api/external/envoy/extensions/filters/http/buffer/v3" v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" - extauth "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/enterprise/options/extauth/v1" "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/faultinjection" "github.com/solo-io/gloo/projects/gloo/pkg/bootstrap" "github.com/solo-io/gloo/projects/gloo/pkg/plugins" @@ -34,54 +23,34 @@ import ( "github.com/solo-io/gloo/projects/gloo/pkg/translator" mock_consul "github.com/solo-io/gloo/projects/gloo/pkg/upstreams/consul/mocks" "github.com/solo-io/gloo/projects/gloo/pkg/utils" - validationutils "github.com/solo-io/gloo/projects/gloo/pkg/utils/validation" gloovalidation "github.com/solo-io/gloo/projects/gloo/pkg/validation" "github.com/solo-io/gloo/test/samples" corev1 "github.com/solo-io/skv2/pkg/api/core.skv2.solo.io/v1" - "github.com/solo-io/solo-kit/pkg/api/v1/clients" "github.com/solo-io/solo-kit/pkg/api/v1/clients/factory" corecache "github.com/solo-io/solo-kit/pkg/api/v1/clients/kube/cache" "github.com/solo-io/solo-kit/pkg/api/v1/clients/memory" "github.com/solo-io/solo-kit/pkg/api/v1/resources/core" - "github.com/solo-io/solo-kit/pkg/api/v2/reporter" - "github.com/solo-io/solo-kit/test/matchers" - k8scorev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" gwv1 "sigs.k8s.io/gateway-api/apis/v1" ) var _ = Describe("RouteOptionsPlugin", func() { var ( - ctx context.Context - cancel context.CancelFunc - sch *runtime.Scheme - authConfigClient extauth.AuthConfigClient - routeOptionClient sologatewayv1.RouteOptionClient - statusReporter reporter.StatusReporter - inputChannels *proxy_syncer.GatewayInputChannels + ctx context.Context + cancel context.CancelFunc - ctrl *gomock.Controller - settings *v1.Settings - registeredPlugins []plugins.Plugin + ctrl *gomock.Controller + settings *v1.Settings + vc gloovalidation.ValidatorConfig ) BeforeEach(func() { ctx, cancel = context.WithCancel(context.Background()) - sch = scheme.NewScheme() resourceClientFactory := &factory.MemoryResourceClientFactory{ Cache: memory.NewInMemoryResourceCache(), } - routeOptionClient, _ = sologatewayv1.NewRouteOptionClient(ctx, resourceClientFactory) - authConfigClient, _ = extauth.NewAuthConfigClient(ctx, resourceClientFactory) - statusClient := statusutils.GetStatusClientForNamespace("gloo-system") - - statusReporter = reporter.NewReporter("gloo-kube-gateway", statusClient, routeOptionClient.BaseClient()) - - inputChannels = proxy_syncer.NewGatewayInputChannels() - ctrl = gomock.NewController(T) kube := fake.NewSimpleClientset() kubeCoreCache, err := corecache.NewKubeCoreCache(context.Background(), kube) @@ -97,14 +66,7 @@ var _ = Describe("RouteOptionsPlugin", func() { KubeClient: kube, KubeCoreCache: kubeCoreCache, } - registeredPlugins = registry.Plugins(opts) - }) - - AfterEach(func() { - cancel() - }) - - FIt("validates a RouteOption with a dummy proxy", func() { + registeredPlugins := registry.Plugins(opts) routeReplacingSanitizer, _ := sanitizer.NewRouteReplacingSanitizer(settings.GetGloo().GetInvalidConfigPolicy()) xdsSanitizer := sanitizer.XdsSanitizers{ sanitizer.NewUpstreamRemovingSanitizer(), @@ -119,182 +81,63 @@ var _ = Describe("RouteOptionsPlugin", func() { pluginRegistry, translator.EnvoyCacheResourcesListToFnvHash, ) - vc := gloovalidation.ValidatorConfig{ + vc = gloovalidation.ValidatorConfig{ Ctx: context.Background(), GlooValidatorConfig: gloovalidation.GlooValidatorConfig{ XdsSanitizer: xdsSanitizer, Translator: translator, }, } - gv := gloovalidation.NewValidator(vc) + }) - deps := []client.Object{svc(), gw(), httpRoute(), attachedRouteOption()} - routeOptionClient.Write(attachedInternal(), clients.WriteOpts{}) - fakeClient := testutils.BuildIndexedFakeClient(deps, gwquery.IterateIndices, rtoptquery.IterateIndices) - gwQueries := testutils.BuildGatewayQueriesWithClient(fakeClient) + AfterEach(func() { + cancel() + }) - k8sGwExtensions, _ := extensions.NewK8sGatewayExtensions(ctx, extensions.K8sGatewayExtensionsFactoryParameters{ - Cl: fakeClient, - Scheme: sch, - AuthConfigClient: authConfigClient, - RouteOptionClient: routeOptionClient, - StatusReporter: statusReporter, - KickXds: inputChannels.Kick, - }) + It("validates a RouteOption with a dummy proxy", func() { + const faultErrorMsg = "Route Error: ProcessingError. Reason: *faultinjection.plugin: invalid abort status code '0', must be in range of [200,600)." + gv := gloovalidation.NewValidator(vc) rtOpt := attachedInternal() - validator := validation.ValidationHelper{ - K8sGwExtensions: k8sGwExtensions, - GatewayQueries: gwQueries, - Cl: fakeClient, - } - params := plugins.Params{ - Ctx: context.Background(), + Ctx: ctx, Snapshot: samples.SimpleGlooSnapshot("gloo-system"), } - proxies, _ := validator.TranslateK8sGatewayProxies(ctx, params.Snapshot, rtOpt) - params.Snapshot.Proxies = proxies + proxies, _ := validation.TranslateK8sGatewayProxies(ctx, params.Snapshot, rtOpt) gv.Sync(ctx, params.Snapshot) rpt, err := gv.ValidateGloo(ctx, proxies[0], rtOpt, false) Expect(err).NotTo(HaveOccurred()) + err = validation.GetSimpleErrorFromGlooValidation(rpt, proxies[0], true) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring(faultErrorMsg)) r := rpt[0] - Expect(r.Proxy).To(Equal(proxies[0])) - Expect(r.ResourceReports).To(Equal(reporter.ResourceReports{})) - Expect(r.ProxyReport).To(matchers.MatchProto(validationutils.MakeReport(proxies[0]))) + proxyResourceReport := r.ResourceReports[proxies[0]] + Expect(proxyResourceReport.Errors.Error()).To(ContainSubstring(faultErrorMsg)) + }) + It("validates a VirtualHostOption with a dummy proxy", func() { + gv := gloovalidation.NewValidator(vc) + + params := plugins.Params{ + Ctx: ctx, + Snapshot: samples.SimpleGlooSnapshot("gloo-system"), + } vhost := attachedVHostInternal() - proxies, _ = validator.TranslateK8sGatewayProxies(ctx, params.Snapshot, vhost) - params.Snapshot.Proxies = proxies + proxies, _ := validation.TranslateK8sGatewayProxies(ctx, params.Snapshot, vhost) gv.Sync(ctx, params.Snapshot) - rpt, err = gv.ValidateGloo(ctx, proxies[0], vhost, false) + rpt, err := gv.ValidateGloo(ctx, proxies[0], vhost, false) + validation.GetSimpleErrorFromGlooValidation(rpt, proxies[0], true) Expect(err).NotTo(HaveOccurred()) - r = rpt[0] - Expect(r.Proxy).To(Equal(proxies[0])) - Expect(r.ResourceReports).To(Equal(reporter.ResourceReports{})) - Expect(r.ProxyReport).To(matchers.MatchProto(validationutils.MakeReport(proxies[0]))) + err = validation.GetSimpleErrorFromGlooValidation(rpt, proxies[0], true) + Expect(err).To(HaveOccurred()) + const bufferErrorMsg = "VirtualHost Error: ProcessingError. Reason: invalid virtual host [vhost]: invalid BufferPerRoute.Buffer: embedded message failed validation | caused by: invalid Buffer.MaxRequestBytes: value is required and must not be nil." + Expect(err.Error()).To(ContainSubstring(bufferErrorMsg)) + r := rpt[0] + proxyResourceReport := r.ResourceReports[proxies[0]] + Expect(proxyResourceReport.Errors.Error()).To(ContainSubstring(bufferErrorMsg)) }) - - // It("validates a RouteOption", func() { - // deps := []client.Object{svc(), gw(), httpRoute(), attachedRouteOption()} - // routeOptionClient.Write(attachedInternal(), clients.WriteOpts{}) - // fakeClient := testutils.BuildIndexedFakeClient(deps, gwquery.IterateIndices, rtoptquery.IterateIndices) - // gwQueries := testutils.BuildGatewayQueriesWithClient(fakeClient) - - // k8sGwExtensions, _ := extensions.NewK8sGatewayExtensions(ctx, extensions.K8sGatewayExtensionsFactoryParameters{ - // Cl: fakeClient, - // Scheme: sch, - // AuthConfigClient: authConfigClient, - // RouteOptionClient: routeOptionClient, - // StatusReporter: statusReporter, - // KickXds: inputChannels.Kick, - // }) - - // rtOpt := attachedInternal() - // validator := validation.ValidationHelper{ - // K8sGwExtensions: k8sGwExtensions, - // GatewayQueries: gwQueries, - // Cl: fakeClient, - // } - // validator.TranslateK8sGatewayProxies(ctx, rtOpt) - // }) }) -func nsPtr(s string) *gwv1.Namespace { - var ns gwv1.Namespace = gwv1.Namespace(s) - return &ns -} - -func portNumPtr(n int32) *gwv1.PortNumber { - var pn gwv1.PortNumber = gwv1.PortNumber(n) - return &pn -} - -func svc() *k8scorev1.Service { - return &k8scorev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "my-svc", - }, - Spec: k8scorev1.ServiceSpec{ - Ports: []k8scorev1.ServicePort{ - { - Port: 8080, - }, - }, - }, - } -} - -func httpRoute() *gwv1.HTTPRoute { - return &gwv1.HTTPRoute{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-route", - Namespace: "default", - }, - Spec: gwv1.HTTPRouteSpec{ - CommonRouteSpec: gwv1.CommonRouteSpec{ - ParentRefs: []gwv1.ParentReference{ - { - Name: "my-gw", - Namespace: nsPtr("default"), - }, - }, - }, - Hostnames: []gwv1.Hostname{ - gwv1.Hostname("example.com"), - }, - Rules: []gwv1.HTTPRouteRule{ - { - BackendRefs: []gwv1.HTTPBackendRef{ - { - BackendRef: gwv1.BackendRef{ - BackendObjectReference: gwv1.BackendObjectReference{ - Name: "my-svc", - Port: portNumPtr(8080), - }, - }, - }, - }, - }, - }, - }, - } -} - -func gw() *gwv1.Gateway { - return &gwv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-gw", - Namespace: "default", - }, - Spec: gwv1.GatewaySpec{ - Listeners: []gwv1.Listener{ - { - Name: "my-http-listener", - Port: gwv1.PortNumber(8080), - Protocol: gwv1.HTTPProtocolType, - // AllowedRoutes: &gwv1.AllowedRoutes{ - // Namespaces: &gwv1.RouteNamespaces{ - // gw - // }, - // }, - }, - }, - }, - } -} - -func attachedVirtualHostOption() *solokubev1.VirtualHostOption { - return &solokubev1.VirtualHostOption{ - ObjectMeta: metav1.ObjectMeta{ - Name: "policy", - Namespace: "default", - }, - Spec: *attachedVHostInternal(), - } -} - func attachedVHostInternal() *sologatewayv1.VirtualHostOption { return &sologatewayv1.VirtualHostOption{ TargetRef: &corev1.PolicyTargetReferenceWithSectionName{ @@ -315,21 +158,6 @@ func attachedVHostInternal() *sologatewayv1.VirtualHostOption { } } -func attachedRouteOption() *solokubev1.RouteOption { - now := metav1.Now() - return &solokubev1.RouteOption{ - TypeMeta: metav1.TypeMeta{ - Kind: sologatewayv1.RouteOptionGVK.Kind, - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "policy", - Namespace: "default", - CreationTimestamp: now, - }, - Spec: *attachedInternal(), - } -} - func attachedInternal() *sologatewayv1.RouteOption { return &sologatewayv1.RouteOption{ Metadata: &core.Metadata{ @@ -346,7 +174,7 @@ func attachedInternal() *sologatewayv1.RouteOption { Faults: &faultinjection.RouteFaults{ Abort: &faultinjection.RouteAbort{ Percentage: 4.19, - HttpStatus: 500, + // HttpStatus: 500, }, }, }, diff --git a/projects/gloo/pkg/syncer/setup/setup_syncer.go b/projects/gloo/pkg/syncer/setup/setup_syncer.go index 599c1af238f..d97752d1863 100644 --- a/projects/gloo/pkg/syncer/setup/setup_syncer.go +++ b/projects/gloo/pkg/syncer/setup/setup_syncer.go @@ -52,12 +52,8 @@ import ( gwtranslator "github.com/solo-io/gloo/projects/gateway/pkg/translator" "github.com/solo-io/gloo/projects/gateway/pkg/utils/metrics" gwvalidation "github.com/solo-io/gloo/projects/gateway/pkg/validation" - "github.com/solo-io/gloo/projects/gateway2/controller/scheme" k8sgwextensions "github.com/solo-io/gloo/projects/gateway2/extensions" - "github.com/solo-io/gloo/projects/gateway2/proxy_syncer" - "github.com/solo-io/gloo/projects/gateway2/query" "github.com/solo-io/gloo/projects/gateway2/status" - k8svalidation "github.com/solo-io/gloo/projects/gateway2/validation" "github.com/solo-io/gloo/projects/gloo/constants" rlv1alpha1 "github.com/solo-io/gloo/projects/gloo/pkg/api/external/solo/ratelimit" v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" @@ -84,9 +80,6 @@ import ( sslutils "github.com/solo-io/gloo/projects/gloo/pkg/utils" "github.com/solo-io/gloo/projects/gloo/pkg/validation" "github.com/solo-io/gloo/projects/gloo/pkg/xds" - ctrl "sigs.k8s.io/controller-runtime" - ctrlzap "sigs.k8s.io/controller-runtime/pkg/log/zap" - metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" ) // TODO: (copied from gateway) switch AcceptAllResourcesByDefault to false after validation has been tested in user environments @@ -863,49 +856,6 @@ func RunGlooWithExtensions(opts bootstrap.Opts, extensions Extensions) error { logger.Debugf("Gateway translation is disabled. Proxies are provided from another source") } - setupLog := ctrl.Log.WithName("setup") - var zapOpts []ctrlzap.Opts - // if cfg.Dev { - // setupLog.Info("starting log in dev mode") - // opts = append(opts, ctrlzap.UseDevMode(true)) - // } - ctrl.SetLogger(ctrlzap.New(zapOpts...)) - - mgrOpts := ctrl.Options{ - Scheme: scheme.NewScheme(), - PprofBindAddress: "127.0.0.1:9099", - // if you change the port here, also change the port "health" in the helmchart. - HealthProbeBindAddress: ":9093", - Metrics: metricsserver.Options{ - BindAddress: ":9092", - }, - } - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), mgrOpts) - if err != nil { - setupLog.Error(err, "unable to start manager") - return err - } - - statusReporter := reporter.NewReporter("gloo-kube-gateway", statusClient, routeOptionClient.BaseClient()) - inputChannels := proxy_syncer.NewGatewayInputChannels() - k8sGwExtensions, err := extensions.K8sGatewayExtensionsFactory(watchOpts.Ctx, k8sgwextensions.K8sGatewayExtensionsFactoryParameters{ - Cl: mgr.GetClient(), - Scheme: mgr.GetScheme(), - AuthConfigClient: authConfigClient, - RouteOptionClient: routeOptionClient, - StatusReporter: statusReporter, - KickXds: inputChannels.Kick, - }) - if err != nil { - setupLog.Error(err, "unable to create k8s gw extensions") - return err - } - k8sValidator := k8svalidation.ValidationHelper{ - K8sGwExtensions: k8sGwExtensions, - GatewayQueries: query.NewData(mgr.GetClient(), mgr.GetScheme()), - Cl: mgr.GetClient(), - } - // filter the list of extensions to only include the rate limit extension for validation syncerValidatorExtensions := []syncer.TranslatorSyncerExtension{} for _, ext := range syncerExtensions { @@ -924,7 +874,6 @@ func RunGlooWithExtensions(opts bootstrap.Opts, extensions Extensions) error { GlooValidator: validator.ValidateGloo, ExtensionValidator: extensionValidator, DisableValidationAgainstPreviousState: disableValidationAgainstPreviousState, - K8sGatewayValidator: k8sValidator, } if gwOpts.Validation != nil { valOpts := gwOpts.Validation @@ -934,23 +883,29 @@ func RunGlooWithExtensions(opts bootstrap.Opts, extensions Extensions) error { } gwValidationSyncer := gwvalidation.NewValidator(validationConfig) + var ( + gwv2StatusSyncer status.GatewayStatusSyncer + gwv2StatusSyncCallback syncer.OnProxiesTranslatedFn + ) // startFuncs represents the set of StartFunc that should be executed at startup // At the moment, the functionality is used minimally. // Overtime, we should break up this large function into smaller StartFunc startFuncs := map[string]StartFunc{} - var ( - gwv2StatusSyncer status.GatewayStatusSyncer - gwv2StatusSyncCallback syncer.OnProxiesTranslatedFn - ) if opts.GlooGateway.EnableK8sGatewayController { gwv2StatusSyncer = status.NewStatusSyncerFactory() gwv2StatusSyncCallback = gwv2StatusSyncer.HandleProxyReports // Share proxyClient and status syncer with the gateway controller - startFuncs["k8s-gateway-controller"] = K8sGatewayControllerStartFunc(proxyClient, k8sGwExtensions, inputChannels, mgr, gwv2StatusSyncer.QueueStatusForProxies) - + startFuncs["k8s-gateway-controller"] = K8sGatewayControllerStartFunc( + proxyClient, + gwv2StatusSyncer.QueueStatusForProxies, + authConfigClient, + routeOptionClient, + virtualHostOptionClient, + statusClient, + ) } translationSync := syncer.NewTranslatorSyncer( diff --git a/projects/gloo/pkg/syncer/setup/start_func.go b/projects/gloo/pkg/syncer/setup/start_func.go index ce1850efc78..ce4c7bebdd5 100644 --- a/projects/gloo/pkg/syncer/setup/start_func.go +++ b/projects/gloo/pkg/syncer/setup/start_func.go @@ -4,14 +4,16 @@ import ( "context" "golang.org/x/sync/errgroup" - "sigs.k8s.io/controller-runtime/pkg/manager" "github.com/solo-io/go-utils/contextutils" + "github.com/solo-io/solo-kit/pkg/api/v1/resources" + "github.com/solo-io/solo-kit/pkg/api/v2/reporter" + gateway "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" "github.com/solo-io/gloo/projects/gateway2/controller" - "github.com/solo-io/gloo/projects/gateway2/extensions" "github.com/solo-io/gloo/projects/gateway2/proxy_syncer" v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" + api "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/enterprise/options/extauth/v1" "github.com/solo-io/gloo/projects/gloo/pkg/bootstrap" "github.com/solo-io/gloo/projects/gloo/pkg/debug" ) @@ -51,10 +53,11 @@ func ExecuteAsynchronousStartFuncs( // K8sGatewayControllerStartFunc returns a StartFunc to run the k8s Gateway controller func K8sGatewayControllerStartFunc( proxyClient v1.ProxyClient, - k8sGatewayExtensions extensions.K8sGatewayExtensions, - inputChannels *proxy_syncer.GatewayInputChannels, - mgr manager.Manager, queueStatusForProxies proxy_syncer.QueueStatusForProxiesFn, + authConfigClient api.AuthConfigClient, + routeOptionClient gateway.RouteOptionClient, + vhOptionClient gateway.VirtualHostOptionClient, + statusClient resources.StatusClient, ) StartFunc { return func(ctx context.Context, opts bootstrap.Opts, extensions Extensions) error { if opts.ProxyDebugServer.Server != nil { @@ -64,15 +67,18 @@ func K8sGatewayControllerStartFunc( opts.ProxyDebugServer.Server.RegisterProxyReader(debug.K8sGatewayTranslation, proxyClient) } + statusReporter := reporter.NewReporter("gloo-kube-gateway", statusClient, routeOptionClient.BaseClient()) return controller.Start(ctx, controller.StartConfig{ - K8sGatewayExtensions: k8sGatewayExtensions, + ExtensionsFactory: extensions.K8sGatewayExtensionsFactory, GlooPluginRegistryFactory: extensions.PluginRegistryFactory, Opts: opts, - Mgr: mgr, - InputChannels: inputChannels, + QueueStatusForProxies: queueStatusForProxies, - ProxyClient: proxyClient, - QueueStatusForProxies: queueStatusForProxies, + ProxyClient: proxyClient, + AuthConfigClient: authConfigClient, + RouteOptionClient: routeOptionClient, + VirtualHostOptionClient: vhOptionClient, + StatusReporter: statusReporter, // Useful for development purposes // At the moment, this is not tied to any user-facing API From b8a55749118918179efd9997ea1f2cc32b5c3546 Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Mon, 6 May 2024 22:01:46 -0500 Subject: [PATCH 07/30] wrap new validation in k8s gw feature gate --- ...ateway-validation-webhook-configuration.yaml | 2 ++ .../validating_admission_webhook.go | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml b/install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml index c966de36a99..bf838c96fe2 100644 --- a/install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml +++ b/install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml @@ -24,10 +24,12 @@ webhooks: path: "/validation" caBundle: "" # update manually or use certgen job or cert-manager's ca-injector rules: +{{- if .Values.kubeGateway.enabled }} - operations: [ "CREATE", "UPDATE" ] apiGroups: ["gateway.solo.io"] apiVersions: ["v1"] resources: ["routeoptions", "virtualhostoptions"] +{{- end }}{{/* if .Values.kubeGateway.enabled */}} - operations: {{ include "gloo.webhookvalidation.operationsForResource" (list "virtualservices" .Values.gateway.validation.webhook.skipDeleteValidationResources) }} apiGroups: ["gateway.solo.io"] apiVersions: ["v1"] diff --git a/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook.go b/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook.go index dba66c42763..b5b12a40b32 100644 --- a/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook.go +++ b/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook.go @@ -17,10 +17,12 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "github.com/solo-io/gloo/projects/gloo/constants" gloov1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/gloosnapshot" "github.com/solo-io/gloo/pkg/utils" + "github.com/solo-io/gloo/pkg/utils/envutils" "go.opencensus.io/stats" "go.opencensus.io/tag" @@ -441,6 +443,11 @@ func (wh *gatewayValidationWebhook) validateGvk(ctx context.Context, gvk schema. var reports *validation.Reports newResourceFunc := gloosnapshot.ApiGvkToHashableResource[gvk] + // check to see if we should perform validation for this GVK + if !wh.shouldValidateGvk(gvk) { + return nil, nil + } + newResource := newResourceFunc() oldResource := newResourceFunc() @@ -475,6 +482,16 @@ func (wh *gatewayValidationWebhook) validateList(ctx context.Context, rawJson [] return reports, nil } +func (wh *gatewayValidationWebhook) shouldValidateGvk(gvk schema.GroupVersionKind) bool { + if gvk == gwv1.RouteOptionGVK || gvk == gwv1.VirtualHostOptionGVK { + // only validate RouteOption and VirtualHostOption resources if K8s Gateway is enabled + return envutils.IsEnvTruthy(constants.GlooGatewayEnableK8sGwControllerEnv) + } + + // no other special considerations at this point, so continue with validation + return true +} + // shouldValidateResource determines if a resource should be validated AND populates `resource` (and `oldResource` if applicable) from the objects[s] in the `admissionRequest` func (wh *gatewayValidationWebhook) shouldValidateResource(ctx context.Context, admissionRequest *v1beta1.AdmissionRequest, resource, oldResource resources.HashableResource) (bool, error) { logger := contextutils.LoggerFrom(ctx) From 62c8e5cf473816acee1c172b4ccd17d127e050bc Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Mon, 6 May 2024 22:28:19 -0500 Subject: [PATCH 08/30] cleanup --- projects/gateway2/controller/start.go | 4 +- projects/gateway2/deployer/deployer_test.go | 21 ++- ...mple-http-route-with-attached-options.yaml | 5 +- .../testutils/validation_test_utils.go | 140 ------------------ .../gloo/pkg/syncer/setup/setup_syncer.go | 4 +- 5 files changed, 22 insertions(+), 152 deletions(-) delete mode 100644 projects/gateway2/validation/testutils/validation_test_utils.go diff --git a/projects/gateway2/controller/start.go b/projects/gateway2/controller/start.go index a5662477187..bf00842c4a0 100644 --- a/projects/gateway2/controller/start.go +++ b/projects/gateway2/controller/start.go @@ -137,12 +137,12 @@ func Start(ctx context.Context, cfg StartConfig) error { Kick: inputChannels.Kick, Extensions: k8sGwExtensions, } - if err := NewBaseGatewayController(ctx, gwCfg); err != nil { + if err = NewBaseGatewayController(ctx, gwCfg); err != nil { setupLog.Error(err, "unable to create controller") return err } - if err := secrets.NewSecretsController(ctx, mgr, inputChannels); err != nil { + if err = secrets.NewSecretsController(ctx, mgr, inputChannels); err != nil { setupLog.Error(err, "unable to create controller") return err } diff --git a/projects/gateway2/deployer/deployer_test.go b/projects/gateway2/deployer/deployer_test.go index 68bbf8627b7..fee5d4dee7c 100644 --- a/projects/gateway2/deployer/deployer_test.go +++ b/projects/gateway2/deployer/deployer_test.go @@ -26,6 +26,8 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/yaml" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" api "sigs.k8s.io/gateway-api/apis/v1" @@ -119,8 +121,11 @@ var _ = Describe("Deployer", func() { ControllerName: wellknown.GatewayControllerName, }, } - var err error - k8sGatewayExt, err = extensions.NewK8sGatewayExtensions(context.TODO(), extensions.K8sGatewayExtensionsFactoryParameters{}) + mgr, err := ctrl.NewManager(&rest.Config{}, ctrl.Options{}) + Expect(err).NotTo(HaveOccurred()) + k8sGatewayExt, err = extensions.NewK8sGatewayExtensions(context.TODO(), extensions.K8sGatewayExtensionsFactoryParameters{ + Mgr: mgr, + }) Expect(err).NotTo(HaveOccurred()) }) @@ -146,7 +151,11 @@ var _ = Describe("Deployer", func() { }) It("support segmenting by release", func() { - k8sGatewayExt, err := extensions.NewK8sGatewayExtensions(context.TODO(), extensions.K8sGatewayExtensionsFactoryParameters{}) + mgr, err := ctrl.NewManager(&rest.Config{}, ctrl.Options{}) + Expect(err).NotTo(HaveOccurred()) + k8sGatewayExt, err := extensions.NewK8sGatewayExtensions(context.TODO(), extensions.K8sGatewayExtensionsFactoryParameters{ + Mgr: mgr, + }) Expect(err).NotTo(HaveOccurred()) d1, err := deployer.NewDeployer(newFakeClientWithObjs(gwc), &deployer.Inputs{ @@ -245,7 +254,11 @@ var _ = Describe("Deployer", func() { var ( defaultGwpName = "default-gateway-params" defaultDeployerInputs = func() *deployer.Inputs { - k8sGatewayExt, err := extensions.NewK8sGatewayExtensions(context.TODO(), extensions.K8sGatewayExtensionsFactoryParameters{}) + mgr, err := ctrl.NewManager(&rest.Config{}, ctrl.Options{}) + Expect(err).NotTo(HaveOccurred()) + k8sGatewayExt, err := extensions.NewK8sGatewayExtensions(context.TODO(), extensions.K8sGatewayExtensionsFactoryParameters{ + Mgr: mgr, + }) Expect(err).NotTo(HaveOccurred()) return &deployer.Inputs{ ControllerName: wellknown.GatewayControllerName, diff --git a/projects/gateway2/examples/example-http-route-with-attached-options.yaml b/projects/gateway2/examples/example-http-route-with-attached-options.yaml index e7bc5c552f0..67e383348aa 100644 --- a/projects/gateway2/examples/example-http-route-with-attached-options.yaml +++ b/projects/gateway2/examples/example-http-route-with-attached-options.yaml @@ -17,7 +17,6 @@ spec: value: /timeout backendRefs: - name: example-svc - namespace: nginx port: 8080 --- apiVersion: gateway.solo.io/v1 @@ -33,14 +32,13 @@ spec: faults: abort: percentage: 100 - #httpStatus: 500 + httpStatus: 500 timeout: 11s --- apiVersion: v1 kind: Service metadata: name: example-svc - namespace: nginx spec: selector: app.kubernetes.io/name: nginx @@ -53,7 +51,6 @@ apiVersion: v1 kind: Pod metadata: name: nginx - namespace: nginx labels: app.kubernetes.io/name: nginx spec: diff --git a/projects/gateway2/validation/testutils/validation_test_utils.go b/projects/gateway2/validation/testutils/validation_test_utils.go deleted file mode 100644 index 5c7248658bb..00000000000 --- a/projects/gateway2/validation/testutils/validation_test_utils.go +++ /dev/null @@ -1,140 +0,0 @@ -package testutils - -import ( - "google.golang.org/protobuf/types/known/wrapperspb" - - sologatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" - solokubev1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1/kube/apis/gateway.solo.io/v1" - "github.com/solo-io/gloo/projects/gateway2/wellknown" - v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" - "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/faultinjection" - corev1 "github.com/solo-io/skv2/pkg/api/core.skv2.solo.io/v1" - "github.com/solo-io/solo-kit/pkg/api/v1/resources/core" - - k8scorev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - gwv1 "sigs.k8s.io/gateway-api/apis/v1" -) - -func nsPtr(s string) *gwv1.Namespace { - var ns gwv1.Namespace = gwv1.Namespace(s) - return &ns -} - -func portNumPtr(n int32) *gwv1.PortNumber { - var pn gwv1.PortNumber = gwv1.PortNumber(n) - return &pn -} - -func svc() *k8scorev1.Service { - return &k8scorev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "my-svc", - }, - Spec: k8scorev1.ServiceSpec{ - Ports: []k8scorev1.ServicePort{ - { - Port: 8080, - }, - }, - }, - } -} - -func httpRoute() *gwv1.HTTPRoute { - return &gwv1.HTTPRoute{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-route", - Namespace: "default", - }, - Spec: gwv1.HTTPRouteSpec{ - CommonRouteSpec: gwv1.CommonRouteSpec{ - ParentRefs: []gwv1.ParentReference{ - { - Name: "my-gw", - Namespace: nsPtr("default"), - }, - }, - }, - Hostnames: []gwv1.Hostname{ - gwv1.Hostname("example.com"), - }, - Rules: []gwv1.HTTPRouteRule{ - { - BackendRefs: []gwv1.HTTPBackendRef{ - { - BackendRef: gwv1.BackendRef{ - BackendObjectReference: gwv1.BackendObjectReference{ - Name: "my-svc", - Port: portNumPtr(8080), - }, - }, - }, - }, - }, - }, - }, - } -} - -func gw() *gwv1.Gateway { - return &gwv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-gw", - Namespace: "default", - }, - Spec: gwv1.GatewaySpec{ - Listeners: []gwv1.Listener{ - { - Name: "my-http-listener", - Port: gwv1.PortNumber(8080), - Protocol: gwv1.HTTPProtocolType, - // AllowedRoutes: &gwv1.AllowedRoutes{ - // Namespaces: &gwv1.RouteNamespaces{ - // gw - // }, - // }, - }, - }, - }, - } -} - -func attachedRouteOption() *solokubev1.RouteOption { - now := metav1.Now() - return &solokubev1.RouteOption{ - TypeMeta: metav1.TypeMeta{ - Kind: sologatewayv1.RouteOptionGVK.Kind, - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "policy", - Namespace: "default", - CreationTimestamp: now, - }, - Spec: *AttachedInternal(), - } -} - -func AttachedInternal() *sologatewayv1.RouteOption { - return &sologatewayv1.RouteOption{ - Metadata: &core.Metadata{ - Name: "policy", - Namespace: "default", - }, - TargetRef: &corev1.PolicyTargetReference{ - Group: gwv1.GroupVersion.Group, - Kind: wellknown.HTTPRouteKind, - Name: "my-route", - Namespace: wrapperspb.String("default"), - }, - Options: &v1.RouteOptions{ - Faults: &faultinjection.RouteFaults{ - Abort: &faultinjection.RouteAbort{ - Percentage: 4.19, - HttpStatus: 500, - }, - }, - }, - } -} diff --git a/projects/gloo/pkg/syncer/setup/setup_syncer.go b/projects/gloo/pkg/syncer/setup/setup_syncer.go index cc30daef0d0..0d0f8530dbb 100644 --- a/projects/gloo/pkg/syncer/setup/setup_syncer.go +++ b/projects/gloo/pkg/syncer/setup/setup_syncer.go @@ -52,7 +52,7 @@ import ( gwtranslator "github.com/solo-io/gloo/projects/gateway/pkg/translator" "github.com/solo-io/gloo/projects/gateway/pkg/utils/metrics" gwvalidation "github.com/solo-io/gloo/projects/gateway/pkg/validation" - k8sgwextensions "github.com/solo-io/gloo/projects/gateway2/extensions" + "github.com/solo-io/gloo/projects/gateway2/extensions" "github.com/solo-io/gloo/projects/gateway2/status" "github.com/solo-io/gloo/projects/gloo/constants" rlv1alpha1 "github.com/solo-io/gloo/projects/gloo/pkg/api/external/solo/ratelimit" @@ -455,7 +455,7 @@ func (s *setupSyncer) Setup(ctx context.Context, kubeCache kube.SharedCache, mem func RunGloo(opts bootstrap.Opts) error { glooExtensions := Extensions{ - K8sGatewayExtensionsFactory: k8sgwextensions.NewK8sGatewayExtensions, + K8sGatewayExtensionsFactory: extensions.NewK8sGatewayExtensions, PluginRegistryFactory: registry.GetPluginRegistryFactory(opts), SyncerExtensions: []syncer.TranslatorSyncerExtensionFactory{ ratelimitExt.NewTranslatorSyncerExtension, From b0906aa44c51fefeb27182101495aa4f7bd9aa9c Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Mon, 6 May 2024 22:33:05 -0500 Subject: [PATCH 09/30] cleanup --- projects/gateway/pkg/validation/validator.go | 110 +++++++++--------- projects/gateway2/controller/gw_controller.go | 1 - .../gateway_http_route_translator.go | 1 - 3 files changed, 55 insertions(+), 57 deletions(-) diff --git a/projects/gateway/pkg/validation/validator.go b/projects/gateway/pkg/validation/validator.go index b85f23eb35f..61fd3467917 100644 --- a/projects/gateway/pkg/validation/validator.go +++ b/projects/gateway/pkg/validation/validator.go @@ -247,61 +247,6 @@ type validationOutput struct { err error } -// the returned error is a multierror that may contain errors from several Edge Proxies, although this depends on collectAllErrors -func (v *validator) translateGlooEdgeProxies( - ctx context.Context, - snapshot *gloov1snap.ApiSnapshot, - collectAllErrors bool, -) ([]*gloov1.Proxy, error) { - var ( - proxies []*gloov1.Proxy - errs error - ) - - gatewaysByProxy := utils.SortedGatewaysByProxyName(snapshot.Gateways) - - // translate all the proxies - for _, gatewayAndProxyName := range gatewaysByProxy { - proxyName := gatewayAndProxyName.Name - gatewayList := gatewayAndProxyName.Gateways - - // Translate the proxy and process the errors - proxy, reports := v.translator.Translate(ctx, proxyName, snapshot, gatewayList) - - err := v.getErrorsFromResourceReports(reports) - - if err != nil { - err = errors.Wrapf(err, couldNotRenderProxy) - errs = multierror.Append(errs, err) - - if !collectAllErrors { - continue - } - } - - // A nil proxy may have been returned if 0 listeners were created - // continue here even if collecting all errors, because the proxy is nil and there is nothing to validate - if proxy == nil { - continue - } - proxies = append(proxies, proxy) - } - - return proxies, errs -} - -// isK8sGatewayProxy returns true if we are evaluating a Policy resource in the context of K8s Gateway API support. -// Currently the only signal needed to make this decision is if the resource being evaluated is a RouteOption -// or a VirthalHostOption, as we only directly evaluate them in the validator to support K8s Gateway API mode. -func isK8sGatewayProxy(res resources.Resource) bool { - switch res.(type) { - case *sologatewayv1.RouteOption, *sologatewayv1.VirtualHostOption: - return true - default: - return false - } -} - // validateProxiesAndExtensions validates a snapshot against the Gloo and Gateway Translations. The supplied snapshot should have either been // modified to contain the resource being validated or in the case of DELETE validation, have the resource in question removed. // This was removed from the main validation loop to allow it to be re-run against the original snapshot. @@ -426,6 +371,61 @@ func (v *validator) validateProxiesAndExtensions(ctx context.Context, snapshot * } } +// the returned error is a multierror that may contain errors from several Edge Proxies, although this depends on collectAllErrors +func (v *validator) translateGlooEdgeProxies( + ctx context.Context, + snapshot *gloov1snap.ApiSnapshot, + collectAllErrors bool, +) ([]*gloov1.Proxy, error) { + var ( + proxies []*gloov1.Proxy + errs error + ) + + gatewaysByProxy := utils.SortedGatewaysByProxyName(snapshot.Gateways) + + // translate all the proxies + for _, gatewayAndProxyName := range gatewaysByProxy { + proxyName := gatewayAndProxyName.Name + gatewayList := gatewayAndProxyName.Gateways + + // Translate the proxy and process the errors + proxy, reports := v.translator.Translate(ctx, proxyName, snapshot, gatewayList) + + err := v.getErrorsFromResourceReports(reports) + + if err != nil { + err = errors.Wrapf(err, couldNotRenderProxy) + errs = multierror.Append(errs, err) + + if !collectAllErrors { + continue + } + } + + // A nil proxy may have been returned if 0 listeners were created + // continue here even if collecting all errors, because the proxy is nil and there is nothing to validate + if proxy == nil { + continue + } + proxies = append(proxies, proxy) + } + + return proxies, errs +} + +// isK8sGatewayProxy returns true if we are evaluating a Policy resource in the context of K8s Gateway API support. +// Currently the only signal needed to make this decision is if the resource being evaluated is a RouteOption +// or a VirthalHostOption, as we only directly evaluate them in the validator to support K8s Gateway API mode. +func isK8sGatewayProxy(res resources.Resource) bool { + switch res.(type) { + case *sologatewayv1.RouteOption, *sologatewayv1.VirtualHostOption: + return true + default: + return false + } +} + // appendProxyErrors appends the errors and from the proxyReport to the (multi)error, passed in by pointer // It returns a boolean to indicate whether the caller should continue processing the next proxy func (v *validator) appendProxyErrors(errs *error, proxyReport *validation.ProxyReport, proxy *gloov1.Proxy, opts *validationOptions) bool { diff --git a/projects/gateway2/controller/gw_controller.go b/projects/gateway2/controller/gw_controller.go index 10c11ea6a63..817603fa27b 100644 --- a/projects/gateway2/controller/gw_controller.go +++ b/projects/gateway2/controller/gw_controller.go @@ -58,7 +58,6 @@ func (r *gatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } log.Info("reconciling gateway", "Gateway", gw.GetObjectMeta()) - // log.Info("reconciling gateway", "GatewayName", gw.GetObjectMeta().GetName(), "GatewayNamespace", gw.GetObjectMeta().GetNamespace()) objs, err := r.deployer.GetObjsToDeploy(ctx, &gw) if err != nil { return ctrl.Result{}, err diff --git a/projects/gateway2/translator/httproute/gateway_http_route_translator.go b/projects/gateway2/translator/httproute/gateway_http_route_translator.go index 5be3cb108d7..358a2b4af78 100644 --- a/projects/gateway2/translator/httproute/gateway_http_route_translator.go +++ b/projects/gateway2/translator/httproute/gateway_http_route_translator.go @@ -91,7 +91,6 @@ func translateGatewayHTTPRouteRulesUtil( } } -// MARK: translate rules func translateGatewayHTTPRouteRule( ctx context.Context, pluginRegistry registry.PluginRegistry, From 2cced1de7b64f8583dabf6c5942497e5a8d9c26d Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Tue, 7 May 2024 09:34:59 -0500 Subject: [PATCH 10/30] cleanup --- projects/gateway2/validation/validator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/gateway2/validation/validator.go b/projects/gateway2/validation/validator.go index 4aefa3697cf..1cdc6a9ff49 100644 --- a/projects/gateway2/validation/validator.go +++ b/projects/gateway2/validation/validator.go @@ -76,7 +76,7 @@ func TranslateK8sGatewayProxies(ctx context.Context, snap *gloosnapshot.ApiSnaps case *sologatewayv1.RouteOption: routes[0].Options = policy.GetOptions() case *sologatewayv1.VirtualHostOption: - aggregateListener.GetAggregateListener().GetHttpResources().VirtualHosts["vhost"].Options = policy.GetOptions() + aggregateListener.GetAggregateListener().GetHttpResources().GetVirtualHosts()["vhost"].Options = policy.GetOptions() } return []*gloov1.Proxy{proxy}, nil From 53d90a5419876a715beb4bd83d6daed6bb560826 Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Tue, 7 May 2024 09:38:55 -0500 Subject: [PATCH 11/30] changelog --- changelog/v1.17.0-beta26/ggv2-validation.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/v1.17.0-beta26/ggv2-validation.yaml diff --git a/changelog/v1.17.0-beta26/ggv2-validation.yaml b/changelog/v1.17.0-beta26/ggv2-validation.yaml new file mode 100644 index 00000000000..22d0c2812c0 --- /dev/null +++ b/changelog/v1.17.0-beta26/ggv2-validation.yaml @@ -0,0 +1,6 @@ +changelog: + - type: NEW_FEATURE + issueLink: https://github.com/solo-io/solo-projects/issues/6063 + resolvesIssue: true + description: | + Adds webhook validation for Gloo Gateway Policies (e.g. RouteOption and VirtualHostOption) when used with Kubernetes Gateway API From 3ca42007bf8459f6404563a002f3cb4b314b8561 Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Tue, 7 May 2024 10:13:14 -0500 Subject: [PATCH 12/30] add note to docs for allowWarnings --- .../solo-io/gloo/projects/gloo/api/v1/settings.proto.sk.md | 2 +- projects/gloo/api/v1/settings.proto | 3 ++- projects/gloo/pkg/api/v1/settings.pb.go | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/content/reference/api/github.com/solo-io/gloo/projects/gloo/api/v1/settings.proto.sk.md b/docs/content/reference/api/github.com/solo-io/gloo/projects/gloo/api/v1/settings.proto.sk.md index 440424f40d9..18da86a4b0c 100644 --- a/docs/content/reference/api/github.com/solo-io/gloo/projects/gloo/api/v1/settings.proto.sk.md +++ b/docs/content/reference/api/github.com/solo-io/gloo/projects/gloo/api/v1/settings.proto.sk.md @@ -886,7 +886,7 @@ options for configuring admission control / validation | `validationWebhookTlsKey` | `string` | Path to TLS Private Key for Kubernetes Validating webhook. Defaults to `/etc/gateway/validation-certs/tls.key`. | | `ignoreGlooValidationFailure` | `bool` | Deprecated: the Gateway and the Gloo pods are now merged together, there are no longer requests made to a Gloo Validation server. When Gateway cannot communicate with Gloo (e.g. Gloo is offline) resources will be rejected by default. Enable the `ignoreGlooValidationFailure` to prevent the Validation server from rejecting resources due to network errors. | | `alwaysAccept` | [.google.protobuf.BoolValue](https://developers.google.com/protocol-buffers/docs/reference/csharp/class/google/protobuf/well-known-types/bool-value) | Always accept resources even if validation produced an error. Validation will still log the error and increment the validation.gateway.solo.io/resources_rejected stat. Currently defaults to true - must be set to `false` to prevent writing invalid resources to storage. | -| `allowWarnings` | [.google.protobuf.BoolValue](https://developers.google.com/protocol-buffers/docs/reference/csharp/class/google/protobuf/well-known-types/bool-value) | Accept resources if validation produced a warning (defaults to true). By setting to false, this means that validation will start rejecting resources that would result in warnings, rather than just those that would result in errors. | +| `allowWarnings` | [.google.protobuf.BoolValue](https://developers.google.com/protocol-buffers/docs/reference/csharp/class/google/protobuf/well-known-types/bool-value) | Accept resources if validation produced a warning (defaults to true). By setting to false, this means that validation will start rejecting resources that would result in warnings, rather than just those that would result in errors. Note that this setting has no impact on Kubernetes Gateway API validation, as warnings will always be allowed in that context. | | `warnRouteShortCircuiting` | [.google.protobuf.BoolValue](https://developers.google.com/protocol-buffers/docs/reference/csharp/class/google/protobuf/well-known-types/bool-value) | Deprecated: See `server_enabled` and consider configuring it to `false` instead. Write a warning to route resources if validation produced a route ordering warning (defaults to false). By setting to true, this means that Gloo will start assigning warnings to resources that would result in route short-circuiting within a virtual host, for example: - prefix routes that make later routes unreachable - regex routes that make later routes unreachable - duplicate matchers. | | `disableTransformationValidation` | [.google.protobuf.BoolValue](https://developers.google.com/protocol-buffers/docs/reference/csharp/class/google/protobuf/well-known-types/bool-value) | Deprecated: See `server_enabled` and consider configuring it to `false` instead. By default gloo will attempt to validate transformations by calling out to a local envoy binary in `validate` mode. Calling this local envoy binary can become slow when done many times during a single validation. Setting this to true will stop gloo from calling out to envoy to validate the transformations, which may speed up the validation time considerably, but may also cause the transformation config to fail after being sent to envoy. When disabling this, ensure that your transformations are valid prior to applying them. | | `validationServerGrpcMaxSizeBytes` | [.google.protobuf.Int32Value](https://developers.google.com/protocol-buffers/docs/reference/csharp/class/google/protobuf/well-known-types/int-32-value) | By default, gRPC validation messages between gateway and gloo pods have a max message size of 100 MB. Setting this value sets the gRPC max message size in bytes for the gloo validation server. This should only be changed if necessary. If not included, the gRPC max message size will be the default of 100 MB. | diff --git a/projects/gloo/api/v1/settings.proto b/projects/gloo/api/v1/settings.proto index fba5d81fb00..a93f25314d7 100644 --- a/projects/gloo/api/v1/settings.proto +++ b/projects/gloo/api/v1/settings.proto @@ -787,7 +787,8 @@ message GatewayOptions { // Accept resources if validation produced a warning (defaults to true). // By setting to false, this means that validation will start rejecting resources that would result - // in warnings, rather than just those that would result in errors. + // in warnings, rather than just those that would result in errors. Note that this setting has no impact on + // Kubernetes Gateway API validation, as warnings will always be allowed in that context. google.protobuf.BoolValue allow_warnings = 7; // Deprecated: See `server_enabled` and consider configuring it to `false` instead. diff --git a/projects/gloo/pkg/api/v1/settings.pb.go b/projects/gloo/pkg/api/v1/settings.pb.go index effd5511638..4795dd3a93f 100644 --- a/projects/gloo/pkg/api/v1/settings.pb.go +++ b/projects/gloo/pkg/api/v1/settings.pb.go @@ -3255,7 +3255,8 @@ type GatewayOptions_ValidationOptions struct { AlwaysAccept *wrappers.BoolValue `protobuf:"bytes,6,opt,name=always_accept,json=alwaysAccept,proto3" json:"always_accept,omitempty"` // Accept resources if validation produced a warning (defaults to true). // By setting to false, this means that validation will start rejecting resources that would result - // in warnings, rather than just those that would result in errors. + // in warnings, rather than just those that would result in errors. Note that this setting has no impact on + // Kubernetes Gateway API validation, as warnings will always be allowed in that context. AllowWarnings *wrappers.BoolValue `protobuf:"bytes,7,opt,name=allow_warnings,json=allowWarnings,proto3" json:"allow_warnings,omitempty"` // Deprecated: See `server_enabled` and consider configuring it to `false` instead. // Write a warning to route resources if validation produced a route ordering warning (defaults to false). From 5c4291912de3c12a432c78c80260644f12140113 Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Tue, 7 May 2024 14:54:05 -0500 Subject: [PATCH 13/30] add kubeGateway flag to validationWebhook, update tests --- .../validating_admission_webhook.go | 40 +++++++-- .../validating_admission_webhook_test.go | 87 ++++++++++++++++--- .../gloo/pkg/syncer/setup/setup_syncer.go | 2 + 3 files changed, 108 insertions(+), 21 deletions(-) diff --git a/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook.go b/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook.go index b5b12a40b32..cf50cbf3fab 100644 --- a/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook.go +++ b/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook.go @@ -17,12 +17,10 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "github.com/solo-io/gloo/projects/gloo/constants" gloov1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/gloosnapshot" "github.com/solo-io/gloo/pkg/utils" - "github.com/solo-io/gloo/pkg/utils/envutils" "go.opencensus.io/stats" "go.opencensus.io/tag" @@ -101,9 +99,19 @@ type WebhookConfig struct { alwaysAccept bool // accept all resources readGatewaysFromAllNamespaces bool webhookNamespace string + kubeGatewayEnabled bool } -func NewWebhookConfig(ctx context.Context, validator validation.Validator, watchNamespaces []string, port int, serverCertPath, serverKeyPath string, alwaysAccept, readGatewaysFromAllNamespaces bool, webhookNamespace string) WebhookConfig { +func NewWebhookConfig( + ctx context.Context, + validator validation.Validator, + watchNamespaces []string, + port int, + serverCertPath, serverKeyPath string, + alwaysAccept, readGatewaysFromAllNamespaces bool, + webhookNamespace string, + kubeGatewayEnabled bool, +) WebhookConfig { return WebhookConfig{ ctx: ctx, validator: validator, @@ -113,7 +121,9 @@ func NewWebhookConfig(ctx context.Context, validator validation.Validator, watch serverKeyPath: serverKeyPath, alwaysAccept: alwaysAccept, readGatewaysFromAllNamespaces: readGatewaysFromAllNamespaces, - webhookNamespace: webhookNamespace} + webhookNamespace: webhookNamespace, + kubeGatewayEnabled: kubeGatewayEnabled, + } } func NewGatewayValidatingWebhook(cfg WebhookConfig) (*http.Server, error) { @@ -126,6 +136,7 @@ func NewGatewayValidatingWebhook(cfg WebhookConfig) (*http.Server, error) { alwaysAccept := cfg.alwaysAccept readGatewaysFromAllNamespaces := cfg.readGatewaysFromAllNamespaces webhookNamespace := cfg.webhookNamespace + kubeGatewayEnabled := cfg.kubeGatewayEnabled certProvider, err := NewCertificateProvider(serverCertPath, serverKeyPath, log.New(&debugLogger{ctx: ctx}, "validation-webhook-certificate-watcher", log.LstdFlags), ctx, 10*time.Second) if err != nil { @@ -139,6 +150,7 @@ func NewGatewayValidatingWebhook(cfg WebhookConfig) (*http.Server, error) { alwaysAccept, readGatewaysFromAllNamespaces, webhookNamespace, + kubeGatewayEnabled, ) mux := http.NewServeMux() @@ -167,6 +179,7 @@ type gatewayValidationWebhook struct { alwaysAccept bool // read only so no races readGatewaysFromAllNamespaces bool // read only so no races webhookNamespace string // read only so no races + kubeGatewayEnabled bool // read only so no races } type AdmissionReviewWithProxies struct { @@ -185,13 +198,24 @@ type AdmissionResponseWithProxies struct { Proxies []*gloov1.Proxy `json:"proxies,omitempty"` } -func NewGatewayValidationHandler(ctx context.Context, validator validation.Validator, watchNamespaces []string, alwaysAccept bool, readGatewaysFromAllNamespaces bool, webhookNamespace string) *gatewayValidationWebhook { - return &gatewayValidationWebhook{ctx: ctx, +func NewGatewayValidationHandler( + ctx context.Context, + validator validation.Validator, + watchNamespaces []string, + alwaysAccept bool, + readGatewaysFromAllNamespaces bool, + webhookNamespace string, + kubeGatewayEnabled bool, +) *gatewayValidationWebhook { + return &gatewayValidationWebhook{ + ctx: ctx, validator: validator, watchNamespaces: watchNamespaces, alwaysAccept: alwaysAccept, readGatewaysFromAllNamespaces: readGatewaysFromAllNamespaces, - webhookNamespace: webhookNamespace} + webhookNamespace: webhookNamespace, + kubeGatewayEnabled: kubeGatewayEnabled, + } } func (wh *gatewayValidationWebhook) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -485,7 +509,7 @@ func (wh *gatewayValidationWebhook) validateList(ctx context.Context, rawJson [] func (wh *gatewayValidationWebhook) shouldValidateGvk(gvk schema.GroupVersionKind) bool { if gvk == gwv1.RouteOptionGVK || gvk == gwv1.VirtualHostOptionGVK { // only validate RouteOption and VirtualHostOption resources if K8s Gateway is enabled - return envutils.IsEnvTruthy(constants.GlooGatewayEnableK8sGwControllerEnv) + return wh.kubeGatewayEnabled } // no other special considerations at this point, so continue with validation diff --git a/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook_test.go b/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook_test.go index cb804529b4c..fe88cf7726a 100644 --- a/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook_test.go +++ b/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook_test.go @@ -54,7 +54,6 @@ var _ = Describe("ValidatingAdmissionWebhook", func() { validator: mv, } srv = httptest.NewServer(wh) - }) setMockFunctions := func() { @@ -138,6 +137,80 @@ var _ = Describe("ValidatingAdmissionWebhook", func() { }, } + FDescribe("GGv2 Policies", func() { + When("kubeGateway disabled", func() { + DescribeTable( + "always accepts", + func(fail bool, crd crd.Crd, gvk schema.GroupVersionKind, op v1beta1.Operation, resourceOrRef interface{}) { + if fail { + setMockFunctions() + } + reviewRequest := makeReviewRequest(srv.URL, crd, gvk, op, resourceOrRef) + res, err := srv.Client().Do(reviewRequest) + Expect(err).NotTo(HaveOccurred()) + + review, err := parseReviewResponse(res) + Expect(err).NotTo(HaveOccurred()) + Expect(review.Response).NotTo(BeNil()) + + Expect(review.Response.Allowed).To(BeTrue()) + Expect(review.Proxies).To(BeEmpty()) + }, + Entry("RouteOption, CREATE, auto-accept = accepted", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), + Entry("RouteOption, UPDATE, auto-accept = accepted", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Update, routeOption), + Entry("VirtualHostOption CREATE, auto-accept = accepted", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), + Entry("VirtualHostOption UPDATE, auto-accept = accepted", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Update, vHostOption), + Entry("RouteOption, CREATE, auto-fail = accepted", true, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), + Entry("RouteOption, UPDATE, auto-fail = accepted", true, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Update, routeOption), + Entry("VirtualHostOption CREATE, auto-fail = accepted", true, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), + Entry("VirtualHostOption UPDATE, auto-fail = accepted", true, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Update, vHostOption), + ) + }) + When("kubeGateway enabled", func() { + JustBeforeEach(func() { + mv = &mockValidator{} + wh = &gatewayValidationWebhook{ + webhookNamespace: "namespace", + ctx: context.TODO(), + validator: mv, + kubeGatewayEnabled: true, + } + srv = httptest.NewServer(wh) + }) + + DescribeTable( + "webhook processes admission requests", + func(allowed bool, crd crd.Crd, gvk schema.GroupVersionKind, op v1beta1.Operation, resourceOrRef interface{}) { + if !allowed { + setMockFunctions() + } + reviewRequest := makeReviewRequest(srv.URL, crd, gvk, op, resourceOrRef) + res, err := srv.Client().Do(reviewRequest) + Expect(err).NotTo(HaveOccurred()) + + review, err := parseReviewResponse(res) + Expect(err).NotTo(HaveOccurred()) + Expect(review.Response).NotTo(BeNil()) + if !allowed { + Expect(review.Response.Allowed).To(BeFalse()) + } else { + Expect(review.Response.Allowed).To(BeTrue()) + } + Expect(review.Proxies).To(BeEmpty()) + }, + Entry("RouteOption, CREATE, auto-accept = accepted", true, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), + Entry("VirtualHostOption CREATE, auto-accept = accepted", true, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), + Entry("RouteOption, CREATE, auto-fail = fail", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), + Entry("VirtualHostOption CREATE, auto-fail = fail", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), + // TODO(Law): add UPDATE tests here, need to handle oldObject similar to the status update tests + // Entry("RouteOption, UPDATE, auto-accept = accepted", true, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Update, routeOption), + // Entry("VirtualHostOption UPDATE, auto-accept = accepted", true, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Update, vHostOption), + // Entry("RouteOption, UPDATE, auto-fail = fail", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Update, routeOption), + // Entry("VirtualHostOption UPDATE, auto-fail = fail", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Update, vHostOption), + ) + }) + }) + DescribeTable("processes admission requests with auto-accept validator", func(crd crd.Crd, gvk schema.GroupVersionKind, op v1beta1.Operation, resourceOrRef interface{}) { reviewRequest := makeReviewRequest(srv.URL, crd, gvk, op, resourceOrRef) res, err := srv.Client().Do(reviewRequest) @@ -159,8 +232,6 @@ var _ = Describe("ValidatingAdmissionWebhook", func() { Entry("upstream, accepted", gloov1.UpstreamCrd, gloov1.UpstreamCrd.GroupVersionKind(), v1beta1.Create, upstream), Entry("upstream deletion, accepted", gloov1.UpstreamCrd, gloov1.UpstreamCrd.GroupVersionKind(), v1beta1.Delete, upstream.GetMetadata().Ref()), Entry("secret deletion, accepted", gloov1.SecretCrd, gloov1.SecretCrd.GroupVersionKind(), v1beta1.Delete, secret.GetMetadata().Ref()), - Entry("route option, accepted", v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), - Entry("virtualHost option, accepted", v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), ) DescribeTable("processes admission requests with auto-fail validator", func(crd crd.Crd, gvk schema.GroupVersionKind, op v1beta1.Operation, resourceOrRef interface{}) { @@ -189,8 +260,6 @@ var _ = Describe("ValidatingAdmissionWebhook", func() { Entry("upstream, rejected", gloov1.UpstreamCrd, gloov1.UpstreamCrd.GroupVersionKind(), v1beta1.Create, upstream), Entry("upstream deletion, rejected", gloov1.UpstreamCrd, gloov1.UpstreamCrd.GroupVersionKind(), v1beta1.Delete, upstream.GetMetadata().Ref()), Entry("secret deletion, rejected", gloov1.SecretCrd, gloov1.SecretCrd.GroupVersionKind(), v1beta1.Delete, secret.GetMetadata().Ref()), - Entry("route option, rejected", v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), - Entry("virtualHost option, rejected", v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), ) DescribeTable("processes status updates with auto-fail validator", func(expectAllowed bool, crd crd.Crd, gvk schema.GroupVersionKind, op v1beta1.Operation, resource resources.InputResource) { @@ -270,10 +339,6 @@ var _ = Describe("ValidatingAdmissionWebhook", func() { Entry("route table update, accepted", true, v1.RouteTableCrd, v1.RouteTableCrd.GroupVersionKind(), v1beta1.Update, routeTable), Entry("upstream create, rejected", false, gloov1.UpstreamCrd, gloov1.UpstreamCrd.GroupVersionKind(), v1beta1.Create, upstream), Entry("upstream update, accepted", true, gloov1.UpstreamCrd, gloov1.UpstreamCrd.GroupVersionKind(), v1beta1.Update, upstream), - Entry("route option create, rejected", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), - Entry("route option update, accepted", true, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Update, routeOption), - Entry("virtualHost create, rejected", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), - Entry("virtualHost update, accepted", true, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Update, vHostOption), ) DescribeTable("processes metadata updates with auto-fail validator", func(expectAllowed bool, crd crd.Crd, gvk schema.GroupVersionKind, op v1beta1.Operation, resource resources.InputResource) { @@ -343,10 +408,6 @@ var _ = Describe("ValidatingAdmissionWebhook", func() { Entry("route table update, rejected", false, v1.RouteTableCrd, v1.RouteTableCrd.GroupVersionKind(), v1beta1.Update, routeTable), Entry("upstream create, rejected", false, gloov1.UpstreamCrd, gloov1.UpstreamCrd.GroupVersionKind(), v1beta1.Create, upstream), Entry("upstream update, rejected", false, gloov1.UpstreamCrd, gloov1.UpstreamCrd.GroupVersionKind(), v1beta1.Update, upstream), - Entry("route option create, rejected", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), - Entry("route option update, rejected", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Update, routeOption), - Entry("virtualHost create, rejected", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), - Entry("virtualHost update, rejected", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Update, vHostOption), ) Context("invalid yaml", func() { diff --git a/projects/gloo/pkg/syncer/setup/setup_syncer.go b/projects/gloo/pkg/syncer/setup/setup_syncer.go index 0d0f8530dbb..ae008beb1c2 100644 --- a/projects/gloo/pkg/syncer/setup/setup_syncer.go +++ b/projects/gloo/pkg/syncer/setup/setup_syncer.go @@ -973,6 +973,7 @@ func RunGlooWithExtensions(opts bootstrap.Opts, extensions Extensions) error { } } + kubeGatewayEnabled := envutils.IsEnvTruthy(constants.GlooGatewayEnableK8sGwControllerEnv) validationWebhook, err := k8sadmission.NewGatewayValidatingWebhook( k8sadmission.NewWebhookConfig( watchOpts.Ctx, @@ -984,6 +985,7 @@ func RunGlooWithExtensions(opts bootstrap.Opts, extensions Extensions) error { gwOpts.Validation.AlwaysAcceptResources, gwOpts.ReadGatewaysFromAllNamespaces, gwOpts.GlooNamespace, + kubeGatewayEnabled, // pass kubeGateway flag to control validation of KubeGateway policies (e.g. RouteOption, VirtualHostOption) ), ) if err != nil { From 5796bd838114d525b8085ef3b747d6bd4afcf13b Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Tue, 7 May 2024 16:37:30 -0500 Subject: [PATCH 14/30] test cleanup --- .../validating_admission_webhook_test.go | 148 +++++++++--------- 1 file changed, 74 insertions(+), 74 deletions(-) diff --git a/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook_test.go b/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook_test.go index fe88cf7726a..3fa426a5245 100644 --- a/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook_test.go +++ b/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook_test.go @@ -137,80 +137,6 @@ var _ = Describe("ValidatingAdmissionWebhook", func() { }, } - FDescribe("GGv2 Policies", func() { - When("kubeGateway disabled", func() { - DescribeTable( - "always accepts", - func(fail bool, crd crd.Crd, gvk schema.GroupVersionKind, op v1beta1.Operation, resourceOrRef interface{}) { - if fail { - setMockFunctions() - } - reviewRequest := makeReviewRequest(srv.URL, crd, gvk, op, resourceOrRef) - res, err := srv.Client().Do(reviewRequest) - Expect(err).NotTo(HaveOccurred()) - - review, err := parseReviewResponse(res) - Expect(err).NotTo(HaveOccurred()) - Expect(review.Response).NotTo(BeNil()) - - Expect(review.Response.Allowed).To(BeTrue()) - Expect(review.Proxies).To(BeEmpty()) - }, - Entry("RouteOption, CREATE, auto-accept = accepted", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), - Entry("RouteOption, UPDATE, auto-accept = accepted", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Update, routeOption), - Entry("VirtualHostOption CREATE, auto-accept = accepted", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), - Entry("VirtualHostOption UPDATE, auto-accept = accepted", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Update, vHostOption), - Entry("RouteOption, CREATE, auto-fail = accepted", true, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), - Entry("RouteOption, UPDATE, auto-fail = accepted", true, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Update, routeOption), - Entry("VirtualHostOption CREATE, auto-fail = accepted", true, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), - Entry("VirtualHostOption UPDATE, auto-fail = accepted", true, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Update, vHostOption), - ) - }) - When("kubeGateway enabled", func() { - JustBeforeEach(func() { - mv = &mockValidator{} - wh = &gatewayValidationWebhook{ - webhookNamespace: "namespace", - ctx: context.TODO(), - validator: mv, - kubeGatewayEnabled: true, - } - srv = httptest.NewServer(wh) - }) - - DescribeTable( - "webhook processes admission requests", - func(allowed bool, crd crd.Crd, gvk schema.GroupVersionKind, op v1beta1.Operation, resourceOrRef interface{}) { - if !allowed { - setMockFunctions() - } - reviewRequest := makeReviewRequest(srv.URL, crd, gvk, op, resourceOrRef) - res, err := srv.Client().Do(reviewRequest) - Expect(err).NotTo(HaveOccurred()) - - review, err := parseReviewResponse(res) - Expect(err).NotTo(HaveOccurred()) - Expect(review.Response).NotTo(BeNil()) - if !allowed { - Expect(review.Response.Allowed).To(BeFalse()) - } else { - Expect(review.Response.Allowed).To(BeTrue()) - } - Expect(review.Proxies).To(BeEmpty()) - }, - Entry("RouteOption, CREATE, auto-accept = accepted", true, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), - Entry("VirtualHostOption CREATE, auto-accept = accepted", true, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), - Entry("RouteOption, CREATE, auto-fail = fail", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), - Entry("VirtualHostOption CREATE, auto-fail = fail", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), - // TODO(Law): add UPDATE tests here, need to handle oldObject similar to the status update tests - // Entry("RouteOption, UPDATE, auto-accept = accepted", true, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Update, routeOption), - // Entry("VirtualHostOption UPDATE, auto-accept = accepted", true, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Update, vHostOption), - // Entry("RouteOption, UPDATE, auto-fail = fail", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Update, routeOption), - // Entry("VirtualHostOption UPDATE, auto-fail = fail", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Update, vHostOption), - ) - }) - }) - DescribeTable("processes admission requests with auto-accept validator", func(crd crd.Crd, gvk schema.GroupVersionKind, op v1beta1.Operation, resourceOrRef interface{}) { reviewRequest := makeReviewRequest(srv.URL, crd, gvk, op, resourceOrRef) res, err := srv.Client().Do(reviewRequest) @@ -532,6 +458,80 @@ var _ = Describe("ValidatingAdmissionWebhook", func() { Expect(err).ToNot(HaveOccurred()) }) }) + + Describe("Kube Gateway API Policy Validation", func() { + When("kubeGateway disabled", func() { + DescribeTable( + "always accepts", + func(fail bool, crd crd.Crd, gvk schema.GroupVersionKind, op v1beta1.Operation, resourceOrRef interface{}) { + if fail { + setMockFunctions() + } + reviewRequest := makeReviewRequest(srv.URL, crd, gvk, op, resourceOrRef) + res, err := srv.Client().Do(reviewRequest) + Expect(err).NotTo(HaveOccurred()) + + review, err := parseReviewResponse(res) + Expect(err).NotTo(HaveOccurred()) + Expect(review.Response).NotTo(BeNil()) + + Expect(review.Response.Allowed).To(BeTrue()) + Expect(review.Proxies).To(BeEmpty()) + }, + Entry("RouteOption, CREATE, auto-accept = accepted", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), + Entry("RouteOption, UPDATE, auto-accept = accepted", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Update, routeOption), + Entry("VirtualHostOption CREATE, auto-accept = accepted", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), + Entry("VirtualHostOption UPDATE, auto-accept = accepted", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Update, vHostOption), + Entry("RouteOption, CREATE, auto-fail = accepted", true, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), + Entry("RouteOption, UPDATE, auto-fail = accepted", true, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Update, routeOption), + Entry("VirtualHostOption CREATE, auto-fail = accepted", true, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), + Entry("VirtualHostOption UPDATE, auto-fail = accepted", true, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Update, vHostOption), + ) + }) + When("kubeGateway enabled", func() { + JustBeforeEach(func() { + mv = &mockValidator{} + wh = &gatewayValidationWebhook{ + webhookNamespace: "namespace", + ctx: context.TODO(), + validator: mv, + kubeGatewayEnabled: true, + } + srv = httptest.NewServer(wh) + }) + + DescribeTable( + "webhook processes admission requests", + func(allowed bool, crd crd.Crd, gvk schema.GroupVersionKind, op v1beta1.Operation, resourceOrRef interface{}) { + if !allowed { + setMockFunctions() + } + reviewRequest := makeReviewRequest(srv.URL, crd, gvk, op, resourceOrRef) + res, err := srv.Client().Do(reviewRequest) + Expect(err).NotTo(HaveOccurred()) + + review, err := parseReviewResponse(res) + Expect(err).NotTo(HaveOccurred()) + Expect(review.Response).NotTo(BeNil()) + if !allowed { + Expect(review.Response.Allowed).To(BeFalse()) + } else { + Expect(review.Response.Allowed).To(BeTrue()) + } + Expect(review.Proxies).To(BeEmpty()) + }, + Entry("RouteOption, CREATE, auto-accept = accepted", true, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), + Entry("VirtualHostOption CREATE, auto-accept = accepted", true, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), + Entry("RouteOption, CREATE, auto-fail = fail", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Create, routeOption), + Entry("VirtualHostOption CREATE, auto-fail = fail", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Create, vHostOption), + // TODO(Law): add UPDATE tests here, need to handle oldObject similar to the status update tests + // Entry("RouteOption, UPDATE, auto-accept = accepted", true, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Update, routeOption), + // Entry("VirtualHostOption UPDATE, auto-accept = accepted", true, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Update, vHostOption), + // Entry("RouteOption, UPDATE, auto-fail = fail", false, v1.RouteOptionCrd, v1.RouteOptionCrd.GroupVersionKind(), v1beta1.Update, routeOption), + // Entry("VirtualHostOption UPDATE, auto-fail = fail", false, v1.VirtualHostOptionCrd, v1.VirtualHostOptionCrd.GroupVersionKind(), v1beta1.Update, vHostOption), + ) + }) + }) }) func makeReviewRequest(url string, crd crd.Crd, gvk schema.GroupVersionKind, operation v1beta1.Operation, resource interface{}) *http.Request { From b8f0bd304cc74acf8d910f2a777332dec3cc700f Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Tue, 7 May 2024 17:12:04 -0500 Subject: [PATCH 15/30] better testing --- projects/gateway2/install.sh | 6 +- .../gateway2/validation/validator_test.go | 69 ++++++++++++++++--- 2 files changed, 61 insertions(+), 14 deletions(-) diff --git a/projects/gateway2/install.sh b/projects/gateway2/install.sh index 57fd8c3713e..e4fab302173 100755 --- a/projects/gateway2/install.sh +++ b/projects/gateway2/install.sh @@ -11,7 +11,5 @@ helm upgrade --install --create-namespace \ --namespace gloo-system gloo \ ./_test/gloo-1.0.0-ci1.tgz \ --set kubeGateway.enabled=true \ - -f - < Date: Tue, 7 May 2024 17:14:49 -0500 Subject: [PATCH 16/30] cleanup install script --- projects/gateway2/install.sh | 4 ---- 1 file changed, 4 deletions(-) diff --git a/projects/gateway2/install.sh b/projects/gateway2/install.sh index e4fab302173..1d05e351ff7 100755 --- a/projects/gateway2/install.sh +++ b/projects/gateway2/install.sh @@ -3,10 +3,6 @@ set -eux ./ci/kind/setup-kind.sh -./projects/gateway2/kind.sh - -kubectl apply -f https://github.com/kubernetes-sigs/gateway-api/releases/download/v1.0.0/standard-install.yaml - helm upgrade --install --create-namespace \ --namespace gloo-system gloo \ ./_test/gloo-1.0.0-ci1.tgz \ From cc1c83a6135831877ac258ca3cd4df8f44c0bee6 Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Tue, 7 May 2024 18:01:44 -0500 Subject: [PATCH 17/30] cleanup and comments --- projects/gateway/pkg/validation/validator.go | 5 +++-- projects/gateway2/validation/validator.go | 6 +++++- projects/gateway2/validation/validator_test.go | 12 ++++++------ 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/projects/gateway/pkg/validation/validator.go b/projects/gateway/pkg/validation/validator.go index 61fd3467917..7c59d0f1339 100644 --- a/projects/gateway/pkg/validation/validator.go +++ b/projects/gateway/pkg/validation/validator.go @@ -323,9 +323,10 @@ func (v *validator) validateProxiesAndExtensions(ctx context.Context, snapshot * // if we are validating K8s Gateway Policy resources, we only want the errors resulting from // the RouteOption/VirtualHostOption, so let's grab that here and skip the more sophisticated - // error aggregation below + // error aggregation below. Note that we do not care about allowWarnings, as they are not + // used in this case, as we do not do a full translation and use a 'dummy' proxy if validatingK8sGateway { - if err = k8sgwvalidation.GetSimpleErrorFromGlooValidation(glooReports, proxy, true); err != nil { + if err = k8sgwvalidation.GetSimpleErrorFromGlooValidation(glooReports, proxy); err != nil { errs = multierror.Append(errs, err) } continue diff --git a/projects/gateway2/validation/validator.go b/projects/gateway2/validation/validator.go index 1cdc6a9ff49..2aad543ef74 100644 --- a/projects/gateway2/validation/validator.go +++ b/projects/gateway2/validation/validator.go @@ -12,6 +12,9 @@ import ( "github.com/solo-io/solo-kit/pkg/api/v1/resources/core" ) +// TranslateK8sGatewayProxies builds an extremely basic "dummy" gloov1.Proxy object with the provided `res` +// attached in the correct location, i.e. if a RouteOption is passed it will be on the Route or if a VirtualHostOption is +// passed it will be on the VirtualHost func TranslateK8sGatewayProxies(ctx context.Context, snap *gloosnapshot.ApiSnapshot, res resources.Resource) ([]*gloov1.Proxy, error) { us := gloov1.NewUpstream("default", "zzz_fake-upstream-for-gloo-validation") us.UpstreamType = &gloov1.Upstream_Static{ @@ -82,10 +85,11 @@ func TranslateK8sGatewayProxies(ctx context.Context, snap *gloosnapshot.ApiSnaps return []*gloov1.Proxy{proxy}, nil } +// GetSimpleErrorFromGlooValidation will get the Errors for the provided `proxy` that exist in the provided `reports` +// This function will return with the first error present if multiple reports exist func GetSimpleErrorFromGlooValidation( reports []*gloovalidation.GlooValidationReport, proxy *gloov1.Proxy, - allowWarnings bool, ) error { var errs error diff --git a/projects/gateway2/validation/validator_test.go b/projects/gateway2/validation/validator_test.go index 4b0e53f9aba..916d9ddab82 100644 --- a/projects/gateway2/validation/validator_test.go +++ b/projects/gateway2/validation/validator_test.go @@ -107,7 +107,7 @@ var _ = Describe("Kube Gateway API Policy Validation Helper", func() { gv.Sync(ctx, params.Snapshot) rpt, err := gv.ValidateGloo(ctx, proxies[0], rtOpt, false) Expect(err).NotTo(HaveOccurred()) - err = validation.GetSimpleErrorFromGlooValidation(rpt, proxies[0], true) + err = validation.GetSimpleErrorFromGlooValidation(rpt, proxies[0]) Expect(err).To(HaveOccurred()) const faultErrorMsg = "Route Error: ProcessingError. Reason: *faultinjection.plugin: invalid abort status code '0', must be in range of [200,600)." Expect(err.Error()).To(ContainSubstring(faultErrorMsg)) @@ -128,11 +128,11 @@ var _ = Describe("Kube Gateway API Policy Validation Helper", func() { gv.Sync(ctx, params.Snapshot) rpt, err := gv.ValidateGloo(ctx, proxies[0], rtOpt, false) Expect(err).NotTo(HaveOccurred()) - err = validation.GetSimpleErrorFromGlooValidation(rpt, proxies[0], true) + err = validation.GetSimpleErrorFromGlooValidation(rpt, proxies[0]) Expect(err).NotTo(HaveOccurred()) r := rpt[0] proxyResourceReport := r.ResourceReports[proxies[0]] - Expect(proxyResourceReport.Errors).To(BeNil()) + Expect(proxyResourceReport.Errors).NotTo(HaveOccurred()) }) It("validates and a rejects a bad VirtualHostOption", func() { @@ -147,7 +147,7 @@ var _ = Describe("Kube Gateway API Policy Validation Helper", func() { gv.Sync(ctx, params.Snapshot) rpt, err := gv.ValidateGloo(ctx, proxies[0], vhost, false) Expect(err).NotTo(HaveOccurred()) - err = validation.GetSimpleErrorFromGlooValidation(rpt, proxies[0], true) + err = validation.GetSimpleErrorFromGlooValidation(rpt, proxies[0]) Expect(err).To(HaveOccurred()) const bufferErrorMsg = "VirtualHost Error: ProcessingError. Reason: invalid virtual host [vhost]: invalid BufferPerRoute.Buffer: embedded message failed validation | caused by: invalid Buffer.MaxRequestBytes: value is required and must not be nil." Expect(err.Error()).To(ContainSubstring(bufferErrorMsg)) @@ -168,11 +168,11 @@ var _ = Describe("Kube Gateway API Policy Validation Helper", func() { gv.Sync(ctx, params.Snapshot) rpt, err := gv.ValidateGloo(ctx, proxies[0], vhost, false) Expect(err).NotTo(HaveOccurred()) - err = validation.GetSimpleErrorFromGlooValidation(rpt, proxies[0], true) + err = validation.GetSimpleErrorFromGlooValidation(rpt, proxies[0]) Expect(err).ToNot(HaveOccurred()) r := rpt[0] proxyResourceReport := r.ResourceReports[proxies[0]] - Expect(proxyResourceReport.Errors).To(BeNil()) + Expect(proxyResourceReport.Errors).NotTo(HaveOccurred()) }) }) From f356a639dabb48a562b836f28f7ac739ddce3930 Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Wed, 8 May 2024 11:39:00 -0500 Subject: [PATCH 18/30] update vhost plugin error msg --- projects/gateway2/validation/validator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/gateway2/validation/validator_test.go b/projects/gateway2/validation/validator_test.go index 916d9ddab82..8ca1a0f9313 100644 --- a/projects/gateway2/validation/validator_test.go +++ b/projects/gateway2/validation/validator_test.go @@ -149,7 +149,7 @@ var _ = Describe("Kube Gateway API Policy Validation Helper", func() { Expect(err).NotTo(HaveOccurred()) err = validation.GetSimpleErrorFromGlooValidation(rpt, proxies[0]) Expect(err).To(HaveOccurred()) - const bufferErrorMsg = "VirtualHost Error: ProcessingError. Reason: invalid virtual host [vhost]: invalid BufferPerRoute.Buffer: embedded message failed validation | caused by: invalid Buffer.MaxRequestBytes: value is required and must not be nil." + const bufferErrorMsg = "VirtualHost Error: ProcessingError. Reason: invalid virtual host [vhost] while processing plugin buffer: invalid BufferPerRoute.Buffer: embedded message failed validation | caused by: invalid Buffer.MaxRequestBytes: value is required and must not be nil." Expect(err.Error()).To(ContainSubstring(bufferErrorMsg)) r := rpt[0] proxyResourceReport := r.ResourceReports[proxies[0]] From 594f0855cf7e67814f6e4349f702c118a17f9fe1 Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Wed, 8 May 2024 16:31:04 -0500 Subject: [PATCH 19/30] remove redundant kubegateway var --- projects/gloo/pkg/syncer/setup/setup_syncer.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/projects/gloo/pkg/syncer/setup/setup_syncer.go b/projects/gloo/pkg/syncer/setup/setup_syncer.go index cb9afd960a5..f4b1d19ca66 100644 --- a/projects/gloo/pkg/syncer/setup/setup_syncer.go +++ b/projects/gloo/pkg/syncer/setup/setup_syncer.go @@ -973,7 +973,6 @@ func RunGlooWithExtensions(opts bootstrap.Opts, extensions Extensions) error { } } - kubeGatewayEnabled := envutils.IsEnvTruthy(constants.GlooGatewayEnableK8sGwControllerEnv) validationWebhook, err := k8sadmission.NewGatewayValidatingWebhook( k8sadmission.NewWebhookConfig( watchOpts.Ctx, @@ -985,7 +984,7 @@ func RunGlooWithExtensions(opts bootstrap.Opts, extensions Extensions) error { gwOpts.Validation.AlwaysAcceptResources, gwOpts.ReadGatewaysFromAllNamespaces, gwOpts.GlooNamespace, - kubeGatewayEnabled, // pass kubeGateway flag to control validation of KubeGateway policies (e.g. RouteOption, VirtualHostOption) + opts.GlooGateway.EnableK8sGatewayController, // ontrols validation of KubeGateway policies (e.g. RouteOption, VirtualHostOption) ), ) if err != nil { From 49ca0f1c9d4cdd1ac64198f25b5142ff5e636945 Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Wed, 8 May 2024 17:35:39 -0500 Subject: [PATCH 20/30] commentary on webhook manifest --- .../templates/5-gateway-validation-webhook-configuration.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml b/install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml index bf838c96fe2..3fa10fa4393 100644 --- a/install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml +++ b/install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml @@ -26,6 +26,10 @@ webhooks: rules: {{- if .Values.kubeGateway.enabled }} - operations: [ "CREATE", "UPDATE" ] + # RouteOption and VirtualHostOption DELETEs are not supported. + # Their validation is currently limited to usage as Kube Gateway API Policies + # and are hermetically validated for semantic correctness only. This means there + # is no validation needed for DELETEs, as a DELETE will never result be semantically invalid apiGroups: ["gateway.solo.io"] apiVersions: ["v1"] resources: ["routeoptions", "virtualhostoptions"] From a63992ad63813ddaaf1f247baaeb951abbb80c4b Mon Sep 17 00:00:00 2001 From: changelog-bot Date: Thu, 9 May 2024 21:06:27 +0000 Subject: [PATCH 21/30] Adding changelog file to new location --- changelog/v1.17.0-beta27/ggv2-validation.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/v1.17.0-beta27/ggv2-validation.yaml diff --git a/changelog/v1.17.0-beta27/ggv2-validation.yaml b/changelog/v1.17.0-beta27/ggv2-validation.yaml new file mode 100644 index 00000000000..22d0c2812c0 --- /dev/null +++ b/changelog/v1.17.0-beta27/ggv2-validation.yaml @@ -0,0 +1,6 @@ +changelog: + - type: NEW_FEATURE + issueLink: https://github.com/solo-io/solo-projects/issues/6063 + resolvesIssue: true + description: | + Adds webhook validation for Gloo Gateway Policies (e.g. RouteOption and VirtualHostOption) when used with Kubernetes Gateway API From 26a5a6fd0a03ca9b19bddd64160fa8c115f88f57 Mon Sep 17 00:00:00 2001 From: changelog-bot Date: Thu, 9 May 2024 21:06:28 +0000 Subject: [PATCH 22/30] Deleting changelog file from old location --- changelog/v1.17.0-beta26/ggv2-validation.yaml | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 changelog/v1.17.0-beta26/ggv2-validation.yaml diff --git a/changelog/v1.17.0-beta26/ggv2-validation.yaml b/changelog/v1.17.0-beta26/ggv2-validation.yaml deleted file mode 100644 index 22d0c2812c0..00000000000 --- a/changelog/v1.17.0-beta26/ggv2-validation.yaml +++ /dev/null @@ -1,6 +0,0 @@ -changelog: - - type: NEW_FEATURE - issueLink: https://github.com/solo-io/solo-projects/issues/6063 - resolvesIssue: true - description: | - Adds webhook validation for Gloo Gateway Policies (e.g. RouteOption and VirtualHostOption) when used with Kubernetes Gateway API From 781597c427a7355385c7e0ca6f3f8d2e6b09db02 Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Fri, 10 May 2024 02:11:40 -0400 Subject: [PATCH 23/30] wip: e2e test for validation webhook --- pkg/utils/kubeutils/kubectl/cli.go | 17 ++++ .../e2e/features/route_options/suite.go | 14 +++- .../e2e/features/virtualhost_options/suite.go | 42 +++++++--- test/kubernetes/e2e/k8sgateway/k8s_gw_test.go | 4 +- .../k8s_gw_test_no_validation_test.go | 77 +++++++++++++++++++ ...teway-no-webhook-validation-test-helm.yaml | 40 ++++++++++ test/kubernetes/testutils/runtime/context.go | 8 +- 7 files changed, 185 insertions(+), 17 deletions(-) create mode 100644 test/kubernetes/e2e/k8sgateway/k8s_gw_test_no_validation_test.go create mode 100644 test/kubernetes/e2e/k8sgateway/manifests/k8s-gateway-no-webhook-validation-test-helm.yaml diff --git a/pkg/utils/kubeutils/kubectl/cli.go b/pkg/utils/kubeutils/kubectl/cli.go index 8f11f747b41..1f2b2240d62 100644 --- a/pkg/utils/kubeutils/kubectl/cli.go +++ b/pkg/utils/kubeutils/kubectl/cli.go @@ -85,6 +85,23 @@ func (c *Cli) Apply(ctx context.Context, content []byte, extraArgs ...string) er Cause() } +// ApplyFileOut applies the resources defined in a file, and returns an error if one occurred +func (c *Cli) ApplyFileWithRunError(ctx context.Context, fileName string, extraArgs ...string) error { + applyArgs := append([]string{"apply", "-f", fileName}, extraArgs...) + + fileInput, err := os.Open(fileName) + if err != nil { + return err + } + defer func() { + _ = fileInput.Close() + }() + + return c.Command(ctx, applyArgs...). + WithStdin(fileInput). + Run() +} + // ApplyFile applies the resources defined in a file, and returns an error if one occurred func (c *Cli) ApplyFile(ctx context.Context, fileName string, extraArgs ...string) error { applyArgs := append([]string{"apply", "-f", fileName}, extraArgs...) diff --git a/test/kubernetes/e2e/features/route_options/suite.go b/test/kubernetes/e2e/features/route_options/suite.go index e3b42cb4236..cd4909e3b15 100644 --- a/test/kubernetes/e2e/features/route_options/suite.go +++ b/test/kubernetes/e2e/features/route_options/suite.go @@ -23,12 +23,20 @@ type testingSuite struct { // testInstallation contains all the metadata/utilities necessary to execute a series of tests // against an installation of Gloo Gateway testInstallation *e2e.TestInstallation + + // validationEnabled tracks if the validating webhook was enabled and will reject invalid resources + validationEnabled bool } -func NewTestingSuite(ctx context.Context, testInst *e2e.TestInstallation) suite.TestingSuite { +func NewTestingSuite( + ctx context.Context, + testInst *e2e.TestInstallation, + validationEnabled bool, +) suite.TestingSuite { return &testingSuite{ - ctx: ctx, - testInstallation: testInst, + ctx: ctx, + testInstallation: testInst, + validationEnabled: validationEnabled, } } diff --git a/test/kubernetes/e2e/features/virtualhost_options/suite.go b/test/kubernetes/e2e/features/virtualhost_options/suite.go index b042325e71d..86e7b31299b 100644 --- a/test/kubernetes/e2e/features/virtualhost_options/suite.go +++ b/test/kubernetes/e2e/features/virtualhost_options/suite.go @@ -30,12 +30,20 @@ type testingSuite struct { // maps test name to a list of manifests to apply before the test manifests map[string][]string + + // validationEnabled tracks if the validating webhook was enabled and will reject invalid resources + validationEnabled bool } -func NewTestingSuite(ctx context.Context, testInst *e2e.TestInstallation) suite.TestingSuite { +func NewTestingSuite( + ctx context.Context, + testInst *e2e.TestInstallation, + validationEnabled bool, +) suite.TestingSuite { return &testingSuite{ - ctx: ctx, - testInstallation: testInst, + ctx: ctx, + testInstallation: testInst, + validationEnabled: validationEnabled, } } @@ -62,8 +70,12 @@ func (s *testingSuite) BeforeTest(suiteName, testName string) { } for _, manifest := range manifests { - err := s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, manifest) - s.NoError(err, "can apply "+manifest) + err := s.testInstallation.Actions.Kubectl().ApplyFileWithRunError(s.ctx, manifest) + if strings.Contains(manifest, "bad") { + s.ErrorContains(err, "Validating *v1.VirtualHostOption failed") + } else { + s.NoError(err, "can apply "+manifest) + } } } @@ -74,6 +86,10 @@ func (s *testingSuite) AfterTest(suiteName, testName string) { } for _, manifest := range manifests { + if strings.Contains(manifest, "bad") { + // this resource was rejected so no need to delete + continue + } err := s.testInstallation.Actions.Kubectl().DeleteFile(s.ctx, manifest) s.NoError(err, "can delete "+manifest) } @@ -99,12 +115,16 @@ func (s *testingSuite) TestConfigureVirtualHostOptions() { } func (s *testingSuite) TestConfigureInvalidVirtualHostOptions() { - // Check status is rejected on bad VirtualHostOption - s.testInstallation.Assertions.EventuallyResourceStatusMatchesState( - s.getterForMeta(&badVirtualHostOptionMeta), - core.Status_Rejected, - defaults.KubeGatewayReporter, - ) + if s.validationEnabled { + // TODO: assert that resource doesn't exist in cluster + } else { + // Check status is rejected on bad VirtualHostOption + s.testInstallation.Assertions.EventuallyResourceStatusMatchesState( + s.getterForMeta(&badVirtualHostOptionMeta), + core.Status_Rejected, + defaults.KubeGatewayReporter, + ) + } } // The goal here is to test the behavior when multiple VHOs target a gateway with multiple listeners and only some diff --git a/test/kubernetes/e2e/k8sgateway/k8s_gw_test.go b/test/kubernetes/e2e/k8sgateway/k8s_gw_test.go index 05876ee81de..8291157095f 100644 --- a/test/kubernetes/e2e/k8sgateway/k8s_gw_test.go +++ b/test/kubernetes/e2e/k8sgateway/k8s_gw_test.go @@ -72,11 +72,11 @@ func TestK8sGateway(t *testing.T) { }) t.Run("RouteOptions", func(t *testing.T) { - suite.Run(t, route_options.NewTestingSuite(ctx, testInstallation)) + suite.Run(t, route_options.NewTestingSuite(ctx, testInstallation, true)) }) t.Run("VirtualHostOptions", func(t *testing.T) { - suite.Run(t, virtualhost_options.NewTestingSuite(ctx, testInstallation)) + suite.Run(t, virtualhost_options.NewTestingSuite(ctx, testInstallation, true)) }) t.Run("Upstreams", func(t *testing.T) { diff --git a/test/kubernetes/e2e/k8sgateway/k8s_gw_test_no_validation_test.go b/test/kubernetes/e2e/k8sgateway/k8s_gw_test_no_validation_test.go new file mode 100644 index 00000000000..0fe04e3e879 --- /dev/null +++ b/test/kubernetes/e2e/k8sgateway/k8s_gw_test_no_validation_test.go @@ -0,0 +1,77 @@ +package k8sgateway_test + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/solo-io/skv2/codegen/util" + "github.com/stretchr/testify/suite" + + "github.com/solo-io/gloo/test/kube2e/helper" + "github.com/solo-io/gloo/test/kubernetes/e2e" + "github.com/solo-io/gloo/test/kubernetes/e2e/features/port_routing" + "github.com/solo-io/gloo/test/kubernetes/e2e/features/route_options" + "github.com/solo-io/gloo/test/kubernetes/e2e/features/virtualhost_options" + "github.com/solo-io/gloo/test/kubernetes/testutils/gloogateway" +) + +// TestK8sGatewayNoValidation executes tests against a K8s Gateway gloo install with validation disabled +func TestK8sGatewayNoValidation(t *testing.T) { + ctx := context.Background() + testCluster := e2e.MustTestCluster() + testInstallation := testCluster.RegisterTestInstallation( + t, + &gloogateway.Context{ + InstallNamespace: "k8s-gw-test", + ValuesManifestFile: filepath.Join(util.MustGetThisDir(), "manifests", "k8s-gateway-no-webhook-validation-test-helm.yaml"), + }, + ) + + testHelper := e2e.MustTestHelper(ctx, testInstallation) + + // create a tmp output directory for generated resources + tempOutputDir, err := os.MkdirTemp("", testInstallation.Metadata.InstallNamespace) + if err != nil { + t.Fatalf("Failed to create temporary directory: %v", err) + } + defer func() { + // Delete the temporary directory after the test completes + if err := os.RemoveAll(tempOutputDir); err != nil { + t.Errorf("Failed to remove temporary directory: %v", err) + } + }() + + // We register the cleanup function _before_ we actually perform the installation. + // This allows us to uninstall Gloo Gateway, in case the original installation only completed partially + t.Cleanup(func() { + if t.Failed() { + testInstallation.PreFailHandler(ctx) + } + + testInstallation.UninstallGlooGateway(ctx, func(ctx context.Context) error { + return testHelper.UninstallGlooAll() + }) + testCluster.UnregisterTestInstallation(testInstallation) + }) + + // Install Gloo Gateway + testInstallation.InstallGlooGateway(ctx, func(ctx context.Context) error { + return testHelper.InstallGloo(ctx, helper.GATEWAY, 5*time.Minute, helper.ExtraArgs("--values", testInstallation.Metadata.ValuesManifestFile)) + }) + + t.Run("RouteOptions", func(t *testing.T) { + suite.Run(t, route_options.NewTestingSuite(ctx, testInstallation, false)) + }) + + t.Run("VirtualHostOptions", func(t *testing.T) { + suite.Run(t, virtualhost_options.NewTestingSuite(ctx, testInstallation, false)) + }) + + t.Run("PortRouting", func(t *testing.T) { + suite.Run(t, port_routing.NewTestingSuite(ctx, testInstallation)) + + }) +} diff --git a/test/kubernetes/e2e/k8sgateway/manifests/k8s-gateway-no-webhook-validation-test-helm.yaml b/test/kubernetes/e2e/k8sgateway/manifests/k8s-gateway-no-webhook-validation-test-helm.yaml new file mode 100644 index 00000000000..a2ad326f895 --- /dev/null +++ b/test/kubernetes/e2e/k8sgateway/manifests/k8s-gateway-no-webhook-validation-test-helm.yaml @@ -0,0 +1,40 @@ +global: + image: + pullPolicy: IfNotPresent + # Note: glooRbac.namespaced settings are not supported with Gloo Gateway https://github.com/solo-io/solo-projects/issues/6064 + # Gateway API fundamentally expects HTTPRoutes and Gateways in any namespace and cross-namespace references to be supported + # Currently we are explicitly disabled namespaced roles for Gloo Gateway tests, but this can be left unset. + glooRbac: + namespaced: false +settings: + # Gloo Gateway requires access to namespaces outside of the install namespace to watch and create Gateway resources + # singleNamespace=false must be set for namespace watch to work correctly. See: https://github.com/solo-io/solo-projects/issues/6058 + singleNamespace: false + create: true + invalidConfigPolicy: + replaceInvalidRoutes: true + invalidRouteResponseCode: 404 + invalidRouteResponseBody: Gloo Gateway has invalid configuration. +gateway: + persistProxySpec: true + logLevel: info + validation: + # this is not relevant for k8s GW + allowWarnings: true + # always accept regardless of validation result + alwaysAcceptResources: true +kubeGateway: + # This is the field that enables the K8s Gateway Integration in Gloo Gateway + enabled: true +gloo: + logLevel: info + disableLeaderElection: true + deployment: + replicas: 1 + livenessProbeEnabled: true +gatewayProxies: + gatewayProxy: + healthyPanicThreshold: 0 +# Disable discovery, not recommended for production +discovery: + enabled: false \ No newline at end of file diff --git a/test/kubernetes/testutils/runtime/context.go b/test/kubernetes/testutils/runtime/context.go index fc1459f34c1..4f92f3b51aa 100644 --- a/test/kubernetes/testutils/runtime/context.go +++ b/test/kubernetes/testutils/runtime/context.go @@ -28,9 +28,15 @@ func NewContext() Context { runSource = PullRequest } + // get clusterName from env var, default to "kind" if not set + clusterName := os.Getenv(testutils.ClusterName) + if clusterName == "" { + clusterName = "kind" + } + return Context{ // ClusterName is derived from the environment variable - ClusterName: os.Getenv(testutils.ClusterName), + ClusterName: clusterName, RunSource: runSource, } From db19e586a31d5203f10b3ed9de56a3f2fc1e6aea Mon Sep 17 00:00:00 2001 From: changelog-bot Date: Mon, 13 May 2024 20:06:03 +0000 Subject: [PATCH 24/30] Adding changelog file to new location --- changelog/v1.17.0-beta28/ggv2-validation.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/v1.17.0-beta28/ggv2-validation.yaml diff --git a/changelog/v1.17.0-beta28/ggv2-validation.yaml b/changelog/v1.17.0-beta28/ggv2-validation.yaml new file mode 100644 index 00000000000..22d0c2812c0 --- /dev/null +++ b/changelog/v1.17.0-beta28/ggv2-validation.yaml @@ -0,0 +1,6 @@ +changelog: + - type: NEW_FEATURE + issueLink: https://github.com/solo-io/solo-projects/issues/6063 + resolvesIssue: true + description: | + Adds webhook validation for Gloo Gateway Policies (e.g. RouteOption and VirtualHostOption) when used with Kubernetes Gateway API From f7544ecb93eebf0117158c115983bfa2d2d960aa Mon Sep 17 00:00:00 2001 From: changelog-bot Date: Mon, 13 May 2024 20:06:03 +0000 Subject: [PATCH 25/30] Deleting changelog file from old location --- changelog/v1.17.0-beta27/ggv2-validation.yaml | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 changelog/v1.17.0-beta27/ggv2-validation.yaml diff --git a/changelog/v1.17.0-beta27/ggv2-validation.yaml b/changelog/v1.17.0-beta27/ggv2-validation.yaml deleted file mode 100644 index 22d0c2812c0..00000000000 --- a/changelog/v1.17.0-beta27/ggv2-validation.yaml +++ /dev/null @@ -1,6 +0,0 @@ -changelog: - - type: NEW_FEATURE - issueLink: https://github.com/solo-io/solo-projects/issues/6063 - resolvesIssue: true - description: | - Adds webhook validation for Gloo Gateway Policies (e.g. RouteOption and VirtualHostOption) when used with Kubernetes Gateway API From 292ccd87c8c2efc41338604b0df64fdeff052fdd Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Mon, 13 May 2024 17:04:43 -0500 Subject: [PATCH 26/30] add assertions for webhook validation testing --- pkg/utils/kubeutils/kubectl/cli.go | 45 +++++++++---------- .../gloo/pkg/syncer/setup/setup_syncer.go | 2 +- .../e2e/features/route_options/suite.go | 9 +--- .../e2e/features/virtualhost_options/suite.go | 33 +++++--------- ...d-vho.yaml => webhook-reject-bad-vho.yaml} | 0 .../e2e/features/virtualhost_options/types.go | 2 +- ...n_test.go => k8s_gw_no_validation_test.go} | 26 +++-------- test/kubernetes/e2e/k8sgateway/k8s_gw_test.go | 9 ++-- .../testutils/assertions/objects.go | 44 ++++++++++++++++++ .../testutils/gloogateway/context.go | 4 ++ 10 files changed, 95 insertions(+), 79 deletions(-) rename test/kubernetes/e2e/features/virtualhost_options/testdata/{bad-vho.yaml => webhook-reject-bad-vho.yaml} (100%) rename test/kubernetes/e2e/k8sgateway/{k8s_gw_test_no_validation_test.go => k8s_gw_no_validation_test.go} (69%) diff --git a/pkg/utils/kubeutils/kubectl/cli.go b/pkg/utils/kubeutils/kubectl/cli.go index 1f2b2240d62..c89049f2e93 100644 --- a/pkg/utils/kubeutils/kubectl/cli.go +++ b/pkg/utils/kubeutils/kubectl/cli.go @@ -85,39 +85,28 @@ func (c *Cli) Apply(ctx context.Context, content []byte, extraArgs ...string) er Cause() } -// ApplyFileOut applies the resources defined in a file, and returns an error if one occurred -func (c *Cli) ApplyFileWithRunError(ctx context.Context, fileName string, extraArgs ...string) error { - applyArgs := append([]string{"apply", "-f", fileName}, extraArgs...) - - fileInput, err := os.Open(fileName) - if err != nil { - return err - } - defer func() { - _ = fileInput.Close() - }() - - return c.Command(ctx, applyArgs...). - WithStdin(fileInput). - Run() -} - // ApplyFile applies the resources defined in a file, and returns an error if one occurred func (c *Cli) ApplyFile(ctx context.Context, fileName string, extraArgs ...string) error { + _, err := c.ApplyFileWithOutput(ctx, fileName, extraArgs...) + return err +} + +// ApplyFileOut applies the resources defined in a file, and returns an error if one occurred +func (c *Cli) ApplyFileWithOutput(ctx context.Context, fileName string, extraArgs ...string) (string, error) { applyArgs := append([]string{"apply", "-f", fileName}, extraArgs...) fileInput, err := os.Open(fileName) if err != nil { - return err + return "", err } defer func() { _ = fileInput.Close() }() - return c.Command(ctx, applyArgs...). + runErr := c.Command(ctx, applyArgs...). WithStdin(fileInput). - Run(). - Cause() + Run() + return runErr.OutputString(), runErr.Cause() } // Delete deletes the resources defined in the bytes, and returns an error if one occurred @@ -131,20 +120,26 @@ func (c *Cli) Delete(ctx context.Context, content []byte, extraArgs ...string) e // DeleteFile deletes the resources defined in a file, and returns an error if one occurred func (c *Cli) DeleteFile(ctx context.Context, fileName string, extraArgs ...string) error { + _, err := c.DeleteFileWithOutput(ctx, fileName, extraArgs...) + return err +} + +// DeleteFile deletes the resources defined in a file, and returns an error if one occurred +func (c *Cli) DeleteFileWithOutput(ctx context.Context, fileName string, extraArgs ...string) (string, error) { applyArgs := append([]string{"delete", "-f", fileName}, extraArgs...) fileInput, err := os.Open(fileName) if err != nil { - return err + return "", err } defer func() { _ = fileInput.Close() }() - return c.Command(ctx, applyArgs...). + runErr := c.Command(ctx, applyArgs...). WithStdin(fileInput). - Run(). - Cause() + Run() + return runErr.OutputString(), runErr.Cause() } // DeleteFileSafe deletes the resources defined in a file, and returns an error if one occurred diff --git a/projects/gloo/pkg/syncer/setup/setup_syncer.go b/projects/gloo/pkg/syncer/setup/setup_syncer.go index f4b1d19ca66..f6599959675 100644 --- a/projects/gloo/pkg/syncer/setup/setup_syncer.go +++ b/projects/gloo/pkg/syncer/setup/setup_syncer.go @@ -984,7 +984,7 @@ func RunGlooWithExtensions(opts bootstrap.Opts, extensions Extensions) error { gwOpts.Validation.AlwaysAcceptResources, gwOpts.ReadGatewaysFromAllNamespaces, gwOpts.GlooNamespace, - opts.GlooGateway.EnableK8sGatewayController, // ontrols validation of KubeGateway policies (e.g. RouteOption, VirtualHostOption) + opts.GlooGateway.EnableK8sGatewayController, // controls validation of KubeGateway policies (e.g. RouteOption, VirtualHostOption) ), ) if err != nil { diff --git a/test/kubernetes/e2e/features/route_options/suite.go b/test/kubernetes/e2e/features/route_options/suite.go index cd4909e3b15..06056e896ee 100644 --- a/test/kubernetes/e2e/features/route_options/suite.go +++ b/test/kubernetes/e2e/features/route_options/suite.go @@ -23,20 +23,15 @@ type testingSuite struct { // testInstallation contains all the metadata/utilities necessary to execute a series of tests // against an installation of Gloo Gateway testInstallation *e2e.TestInstallation - - // validationEnabled tracks if the validating webhook was enabled and will reject invalid resources - validationEnabled bool } func NewTestingSuite( ctx context.Context, testInst *e2e.TestInstallation, - validationEnabled bool, ) suite.TestingSuite { return &testingSuite{ - ctx: ctx, - testInstallation: testInst, - validationEnabled: validationEnabled, + ctx: ctx, + testInstallation: testInst, } } diff --git a/test/kubernetes/e2e/features/virtualhost_options/suite.go b/test/kubernetes/e2e/features/virtualhost_options/suite.go index 86e7b31299b..f11fb6ff59d 100644 --- a/test/kubernetes/e2e/features/virtualhost_options/suite.go +++ b/test/kubernetes/e2e/features/virtualhost_options/suite.go @@ -30,20 +30,15 @@ type testingSuite struct { // maps test name to a list of manifests to apply before the test manifests map[string][]string - - // validationEnabled tracks if the validating webhook was enabled and will reject invalid resources - validationEnabled bool } func NewTestingSuite( ctx context.Context, testInst *e2e.TestInstallation, - validationEnabled bool, ) suite.TestingSuite { return &testingSuite{ - ctx: ctx, - testInstallation: testInst, - validationEnabled: validationEnabled, + ctx: ctx, + testInstallation: testInst, } } @@ -70,12 +65,8 @@ func (s *testingSuite) BeforeTest(suiteName, testName string) { } for _, manifest := range manifests { - err := s.testInstallation.Actions.Kubectl().ApplyFileWithRunError(s.ctx, manifest) - if strings.Contains(manifest, "bad") { - s.ErrorContains(err, "Validating *v1.VirtualHostOption failed") - } else { - s.NoError(err, "can apply "+manifest) - } + output, err := s.testInstallation.Actions.Kubectl().ApplyFileWithOutput(s.ctx, manifest) + s.testInstallation.Assertions.ExpectObjectAdmitted(manifest, err, output, "Validating *v1.VirtualHostOption failed") } } @@ -86,12 +77,8 @@ func (s *testingSuite) AfterTest(suiteName, testName string) { } for _, manifest := range manifests { - if strings.Contains(manifest, "bad") { - // this resource was rejected so no need to delete - continue - } - err := s.testInstallation.Actions.Kubectl().DeleteFile(s.ctx, manifest) - s.NoError(err, "can delete "+manifest) + output, err := s.testInstallation.Actions.Kubectl().DeleteFileWithOutput(s.ctx, manifest) + s.testInstallation.Assertions.ExpectObjectDeleted(manifest, err, output) } } @@ -115,8 +102,12 @@ func (s *testingSuite) TestConfigureVirtualHostOptions() { } func (s *testingSuite) TestConfigureInvalidVirtualHostOptions() { - if s.validationEnabled { - // TODO: assert that resource doesn't exist in cluster + if !s.testInstallation.Metadata.ValidationAlwaysAccept { + s.testInstallation.Assertions.ExpectGlooObjectNotExist( + s.ctx, + s.getterForMeta(&badVirtualHostOptionMeta), + &badVirtualHostOptionMeta, + ) } else { // Check status is rejected on bad VirtualHostOption s.testInstallation.Assertions.EventuallyResourceStatusMatchesState( diff --git a/test/kubernetes/e2e/features/virtualhost_options/testdata/bad-vho.yaml b/test/kubernetes/e2e/features/virtualhost_options/testdata/webhook-reject-bad-vho.yaml similarity index 100% rename from test/kubernetes/e2e/features/virtualhost_options/testdata/bad-vho.yaml rename to test/kubernetes/e2e/features/virtualhost_options/testdata/webhook-reject-bad-vho.yaml diff --git a/test/kubernetes/e2e/features/virtualhost_options/types.go b/test/kubernetes/e2e/features/virtualhost_options/types.go index e02b62c95bc..a6507369b65 100644 --- a/test/kubernetes/e2e/features/virtualhost_options/types.go +++ b/test/kubernetes/e2e/features/virtualhost_options/types.go @@ -18,7 +18,7 @@ var ( basicVhOManifest = filepath.Join(util.MustGetThisDir(), "testdata", "basic-vho.yaml") sectionNameVhOManifest = filepath.Join(util.MustGetThisDir(), "testdata", "section-name-vho.yaml") extraVhOManifest = filepath.Join(util.MustGetThisDir(), "testdata", "extra-vho.yaml") - badVhOManifest = filepath.Join(util.MustGetThisDir(), "testdata", "bad-vho.yaml") + badVhOManifest = filepath.Join(util.MustGetThisDir(), "testdata", "webhook-reject-bad-vho.yaml") // When we apply the setup file, we expect resources to be created with this metadata glooProxyObjectMeta = metav1.ObjectMeta{ diff --git a/test/kubernetes/e2e/k8sgateway/k8s_gw_test_no_validation_test.go b/test/kubernetes/e2e/k8sgateway/k8s_gw_no_validation_test.go similarity index 69% rename from test/kubernetes/e2e/k8sgateway/k8s_gw_test_no_validation_test.go rename to test/kubernetes/e2e/k8sgateway/k8s_gw_no_validation_test.go index 0fe04e3e879..2b6b37a2f97 100644 --- a/test/kubernetes/e2e/k8sgateway/k8s_gw_test_no_validation_test.go +++ b/test/kubernetes/e2e/k8sgateway/k8s_gw_no_validation_test.go @@ -2,7 +2,6 @@ package k8sgateway_test import ( "context" - "os" "path/filepath" "testing" "time" @@ -21,29 +20,17 @@ import ( // TestK8sGatewayNoValidation executes tests against a K8s Gateway gloo install with validation disabled func TestK8sGatewayNoValidation(t *testing.T) { ctx := context.Background() - testCluster := e2e.MustTestCluster() - testInstallation := testCluster.RegisterTestInstallation( + testInstallation := e2e.CreateTestInstallation( t, &gloogateway.Context{ - InstallNamespace: "k8s-gw-test", - ValuesManifestFile: filepath.Join(util.MustGetThisDir(), "manifests", "k8s-gateway-no-webhook-validation-test-helm.yaml"), + InstallNamespace: "k8s-gw-test-no-validation", + ValuesManifestFile: filepath.Join(util.MustGetThisDir(), "manifests", "k8s-gateway-no-webhook-validation-test-helm.yaml"), + ValidationAlwaysAccept: true, }, ) testHelper := e2e.MustTestHelper(ctx, testInstallation) - // create a tmp output directory for generated resources - tempOutputDir, err := os.MkdirTemp("", testInstallation.Metadata.InstallNamespace) - if err != nil { - t.Fatalf("Failed to create temporary directory: %v", err) - } - defer func() { - // Delete the temporary directory after the test completes - if err := os.RemoveAll(tempOutputDir); err != nil { - t.Errorf("Failed to remove temporary directory: %v", err) - } - }() - // We register the cleanup function _before_ we actually perform the installation. // This allows us to uninstall Gloo Gateway, in case the original installation only completed partially t.Cleanup(func() { @@ -54,7 +41,6 @@ func TestK8sGatewayNoValidation(t *testing.T) { testInstallation.UninstallGlooGateway(ctx, func(ctx context.Context) error { return testHelper.UninstallGlooAll() }) - testCluster.UnregisterTestInstallation(testInstallation) }) // Install Gloo Gateway @@ -63,11 +49,11 @@ func TestK8sGatewayNoValidation(t *testing.T) { }) t.Run("RouteOptions", func(t *testing.T) { - suite.Run(t, route_options.NewTestingSuite(ctx, testInstallation, false)) + suite.Run(t, route_options.NewTestingSuite(ctx, testInstallation)) }) t.Run("VirtualHostOptions", func(t *testing.T) { - suite.Run(t, virtualhost_options.NewTestingSuite(ctx, testInstallation, false)) + suite.Run(t, virtualhost_options.NewTestingSuite(ctx, testInstallation)) }) t.Run("PortRouting", func(t *testing.T) { diff --git a/test/kubernetes/e2e/k8sgateway/k8s_gw_test.go b/test/kubernetes/e2e/k8sgateway/k8s_gw_test.go index bff9d5b5a44..991fb1f3e79 100644 --- a/test/kubernetes/e2e/k8sgateway/k8s_gw_test.go +++ b/test/kubernetes/e2e/k8sgateway/k8s_gw_test.go @@ -30,8 +30,9 @@ func TestK8sGateway(t *testing.T) { testInstallation := e2e.CreateTestInstallation( t, &gloogateway.Context{ - InstallNamespace: "k8s-gw-test", - ValuesManifestFile: filepath.Join(util.MustGetThisDir(), "manifests", "k8s-gateway-test-helm.yaml"), + InstallNamespace: "k8s-gw-test", + ValuesManifestFile: filepath.Join(util.MustGetThisDir(), "manifests", "k8s-gateway-test-helm.yaml"), + ValidationAlwaysAccept: false, }, ) @@ -59,11 +60,11 @@ func TestK8sGateway(t *testing.T) { }) t.Run("RouteOptions", func(t *testing.T) { - suite.Run(t, route_options.NewTestingSuite(ctx, testInstallation, true)) + suite.Run(t, route_options.NewTestingSuite(ctx, testInstallation)) }) t.Run("VirtualHostOptions", func(t *testing.T) { - suite.Run(t, virtualhost_options.NewTestingSuite(ctx, testInstallation, true)) + suite.Run(t, virtualhost_options.NewTestingSuite(ctx, testInstallation)) }) t.Run("Upstreams", func(t *testing.T) { diff --git a/test/kubernetes/testutils/assertions/objects.go b/test/kubernetes/testutils/assertions/objects.go index 13ceeed779c..3a33d3678cd 100644 --- a/test/kubernetes/testutils/assertions/objects.go +++ b/test/kubernetes/testutils/assertions/objects.go @@ -3,15 +3,20 @@ package assertions import ( "context" "fmt" + "strings" "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" . "github.com/onsi/gomega" + "github.com/solo-io/gloo/test/helpers" + "github.com/solo-io/solo-kit/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" ) +const WebhookReject = "webhook-reject" + func (p *Provider) EventuallyObjectsExist(ctx context.Context, objects ...client.Object) { for _, o := range objects { p.Gomega.Eventually(ctx, func(innerG Gomega) { @@ -42,3 +47,42 @@ func (p *Provider) ExpectNamespaceNotExist(ctx context.Context, ns string) { _, err := p.clusterContext.Clientset.CoreV1().Namespaces().Get(ctx, ns, metav1.GetOptions{}) p.Gomega.Expect(apierrors.IsNotFound(err)).To(BeTrue(), fmt.Sprintf("namespace %s should not be found in cluster", ns)) } + +func (p *Provider) ExpectGlooObjectNotExist(ctx context.Context, getter helpers.InputResourceGetter, meta *metav1.ObjectMeta) { + _, err := getter() + p.Gomega.Expect(errors.IsNotExist(err)).To(BeTrue(), fmt.Sprintf("obj %s.%s should not be found in cluster", meta.GetName(), meta.GetNamespace())) +} + +// ExpectObjectAdmitted should be used when applying Policy objects that are subject to the Gloo Gateway Validation Webhook +// If the testInstallation has validation enabled and the manifest contains a known substring (e.g. `webhook-reject`) +// we expect the application to fail, with an expected error substring supplied as `expectedOutput` +func (p *Provider) ExpectObjectAdmitted(manifest string, err error, actualOutput, expectedOutput string) { + if p.glooGatewayContext.ValidationAlwaysAccept { + p.Assert.NoError(err, "can apply "+manifest) + return + } + + if strings.Contains(manifest, WebhookReject) { + // when validation is enforced (i.e. does NOT always accept), an apply should result in an error + // and the output from the command should contain a validation failure message + p.Assert.Error(err, "got error when applying "+manifest) + p.Assert.Contains(actualOutput, expectedOutput, "apply failed with expected message for "+manifest) + } else { + p.Assert.NoError(err, "can apply "+manifest) + } +} + +func (p *Provider) ExpectObjectDeleted(manifest string, err error, actualOutput string) { + if p.glooGatewayContext.ValidationAlwaysAccept { + p.Assert.NoError(err, "can delete "+manifest) + return + } + + if strings.Contains(manifest, WebhookReject) { + // when validation is enforced (i.e. does NOT always accept), a delete should result in an error and "not found" in the output + p.Assert.Error(err, "delete failed for "+manifest) + p.Assert.Contains(actualOutput, "NotFound") + } else { + p.Assert.NoError(err, "can delete "+manifest) + } +} diff --git a/test/kubernetes/testutils/gloogateway/context.go b/test/kubernetes/testutils/gloogateway/context.go index ab7da6ab182..b7521601b99 100644 --- a/test/kubernetes/testutils/gloogateway/context.go +++ b/test/kubernetes/testutils/gloogateway/context.go @@ -5,4 +5,8 @@ type Context struct { InstallNamespace string ValuesManifestFile string + + // whether or not the validation webhook is configured to always accept resources, + // i.e. if this is set to true, the webhook will accept regardless of errors found during validation + ValidationAlwaysAccept bool } From 1ac7920881475ff80553d48a0603d93d8c84dc96 Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Mon, 13 May 2024 17:08:56 -0500 Subject: [PATCH 27/30] remove redundant clustername fallback --- test/kubernetes/testutils/runtime/context.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/test/kubernetes/testutils/runtime/context.go b/test/kubernetes/testutils/runtime/context.go index 4f92f3b51aa..fc1459f34c1 100644 --- a/test/kubernetes/testutils/runtime/context.go +++ b/test/kubernetes/testutils/runtime/context.go @@ -28,15 +28,9 @@ func NewContext() Context { runSource = PullRequest } - // get clusterName from env var, default to "kind" if not set - clusterName := os.Getenv(testutils.ClusterName) - if clusterName == "" { - clusterName = "kind" - } - return Context{ // ClusterName is derived from the environment variable - ClusterName: clusterName, + ClusterName: os.Getenv(testutils.ClusterName), RunSource: runSource, } From eb12f4ce71fd0b78b362303124c2473122e7f80f Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Mon, 13 May 2024 17:18:00 -0500 Subject: [PATCH 28/30] add TestK8sGatewayNoValidation to CI --- .github/workflows/pr-kubernetes-tests.yaml | 2 +- pkg/utils/kubeutils/kubectl/cli.go | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pr-kubernetes-tests.yaml b/.github/workflows/pr-kubernetes-tests.yaml index 7e55ddc56e4..462c4e8b0dd 100644 --- a/.github/workflows/pr-kubernetes-tests.yaml +++ b/.github/workflows/pr-kubernetes-tests.yaml @@ -71,7 +71,7 @@ jobs: kubectl-version: 'v1.28.4' helm-version: 'v3.13.2' go-test-args: '-v -timeout=25m' - go-test-run-regex: '(^TestK8sGatewayIstioAutoMtls$$|^TestK8sGatewayIstio$$|^TestGlooGatewayEdgeGateway$$|^TestGlooctlIstioInjectEdgeApiGateway$$|^TestAutomtlsIstioEdgeApisGateway$$|^TestIstioEdgeApiGateway$$)' + go-test-run-regex: '(^TestK8sGatewayIstioAutoMtls$$|^TestK8sGatewayIstio$$|^TestGlooGatewayEdgeGateway$$|^TestGlooctlIstioInjectEdgeApiGateway$$|^TestAutomtlsIstioEdgeApisGateway$$|^TestIstioEdgeApiGateway$$|^TestK8sGatewayNoValidation$$)' steps: - id: auto-succeed-tests diff --git a/pkg/utils/kubeutils/kubectl/cli.go b/pkg/utils/kubeutils/kubectl/cli.go index c89049f2e93..a3c56be52de 100644 --- a/pkg/utils/kubeutils/kubectl/cli.go +++ b/pkg/utils/kubeutils/kubectl/cli.go @@ -91,7 +91,8 @@ func (c *Cli) ApplyFile(ctx context.Context, fileName string, extraArgs ...strin return err } -// ApplyFileOut applies the resources defined in a file, and returns an error if one occurred +// ApplyFileWithOutput applies the resources defined in a file, +// if an error occurred, it will be returned along with the output of the command func (c *Cli) ApplyFileWithOutput(ctx context.Context, fileName string, extraArgs ...string) (string, error) { applyArgs := append([]string{"apply", "-f", fileName}, extraArgs...) @@ -124,7 +125,8 @@ func (c *Cli) DeleteFile(ctx context.Context, fileName string, extraArgs ...stri return err } -// DeleteFile deletes the resources defined in a file, and returns an error if one occurred +// DeleteFileWithOutput deletes the resources defined in a file, +// if an error occurred, it will be returned along with the output of the command func (c *Cli) DeleteFileWithOutput(ctx context.Context, fileName string, extraArgs ...string) (string, error) { applyArgs := append([]string{"delete", "-f", fileName}, extraArgs...) From d5b7bb6bfaafb6069c41f70edb87fc5865c376c7 Mon Sep 17 00:00:00 2001 From: Lawrence Gadban Date: Mon, 13 May 2024 18:04:56 -0500 Subject: [PATCH 29/30] better documentation for validation-based assertions --- test/kubernetes/testutils/assertions/README.md | 14 +++++++++++++- test/kubernetes/testutils/assertions/objects.go | 2 -- test/kubernetes/testutils/assertions/wellknown.go | 6 ++++++ 3 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 test/kubernetes/testutils/assertions/wellknown.go diff --git a/test/kubernetes/testutils/assertions/README.md b/test/kubernetes/testutils/assertions/README.md index 7610c9bd111..a74a399d1fa 100644 --- a/test/kubernetes/testutils/assertions/README.md +++ b/test/kubernetes/testutils/assertions/README.md @@ -2,4 +2,16 @@ If you intend to introduce a new assertion, please follow this approach: - We want to avoid writing generic assertions, that are specific to certain tests. Assertions should contain no custom logic, and instead support dependency injection. -- If you are unsure if an assertion is generic, start by adding it directly to your test, and then you can make it more generic in a follow-up. \ No newline at end of file +- If you are unsure if an assertion is generic, start by adding it directly to your test, and then you can make it more generic in a follow-up. + +# Conventions + +## Contextual Assertions + +Some assertions provided in this package may consider the [Gloo Gateway context](./provider.go#L25) for conditional logic based on metadata about the installation being asserted against. +One example is asserting behavior differently if the validating webhook is installed and configured to reject certain resources. + +## Well Known Manifests + +Additionally, some assertions may rely on [well-known strings](./wellknown.go) in the manifest filenames for conditional logic. +As mentioned above, an example is asserting behavior differently if a manifest is expected to be rejected by the validating webhook. diff --git a/test/kubernetes/testutils/assertions/objects.go b/test/kubernetes/testutils/assertions/objects.go index 3a33d3678cd..17590c755c5 100644 --- a/test/kubernetes/testutils/assertions/objects.go +++ b/test/kubernetes/testutils/assertions/objects.go @@ -15,8 +15,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -const WebhookReject = "webhook-reject" - func (p *Provider) EventuallyObjectsExist(ctx context.Context, objects ...client.Object) { for _, o := range objects { p.Gomega.Eventually(ctx, func(innerG Gomega) { diff --git a/test/kubernetes/testutils/assertions/wellknown.go b/test/kubernetes/testutils/assertions/wellknown.go new file mode 100644 index 00000000000..41d9f65f4e2 --- /dev/null +++ b/test/kubernetes/testutils/assertions/wellknown.go @@ -0,0 +1,6 @@ +package assertions + +// WebhookReject is a well-known string that should be placed in the filename of any manifest which should be rejected +// by the Gloo Gateway validating webhook if it is enabled and configured to reject on errors. This acts as signal +// for any assertion which needs to know if the manifest is expected to not be admitted to the cluster. +const WebhookReject = "webhook-reject" From 664ef7a102c4629720e573875bc3533ad400dedd Mon Sep 17 00:00:00 2001 From: Sam Heilbron Date: Wed, 15 May 2024 14:12:20 -0700 Subject: [PATCH 30/30] proper separator in regex --- .github/workflows/pr-kubernetes-tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr-kubernetes-tests.yaml b/.github/workflows/pr-kubernetes-tests.yaml index 5a56d0b9f71..069fc8c49c3 100644 --- a/.github/workflows/pr-kubernetes-tests.yaml +++ b/.github/workflows/pr-kubernetes-tests.yaml @@ -89,7 +89,7 @@ jobs: kubectl-version: 'v1.28.4' helm-version: 'v3.13.2' go-test-args: '-v -timeout=25m' - go-test-run-regex: '(^TestK8sGatewayIstio$$|^TestGlooGatewayEdgeGateway$$|^TestGlooctlIstioInjectEdgeApiGateway$$,^TestK8sGatewayNoValidation$$)' + go-test-run-regex: '(^TestK8sGatewayIstio$$|^TestGlooGatewayEdgeGateway$$|^TestGlooctlIstioInjectEdgeApiGateway$$|^TestK8sGatewayNoValidation$$)' steps: - id: auto-succeed-tests