diff --git a/.github/workflows/pr-kubernetes-tests.yaml b/.github/workflows/pr-kubernetes-tests.yaml index 630820a6780..c2a2f620199 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$$)' + go-test-run-regex: '(^TestK8sGatewayIstio$$|^TestGlooGatewayEdgeGateway$$|^TestGlooctlIstioInjectEdgeApiGateway$$|^TestK8sGatewayNoValidation$$)' steps: - id: auto-succeed-tests 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 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/install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml b/install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml index c7e0c7cd1b3..22af9833e46 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,16 @@ webhooks: path: "/validation" caBundle: "" # update manually or use certgen job or cert-manager's ca-injector 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"] +{{- 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/pkg/utils/kubeutils/kubectl/cli.go b/pkg/utils/kubeutils/kubectl/cli.go index 8f11f747b41..a3c56be52de 100644 --- a/pkg/utils/kubeutils/kubectl/cli.go +++ b/pkg/utils/kubeutils/kubectl/cli.go @@ -87,20 +87,27 @@ func (c *Cli) Apply(ctx context.Context, content []byte, extraArgs ...string) er // 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 +} + +// 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...) 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 @@ -114,20 +121,27 @@ 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 +} + +// 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...) 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/gateway/pkg/services/k8sadmission/validating_admission_webhook.go b/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook.go index 003884227af..cf50cbf3fab 100644 --- a/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook.go +++ b/projects/gateway/pkg/services/k8sadmission/validating_admission_webhook.go @@ -99,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, @@ -111,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) { @@ -124,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 { @@ -137,6 +150,7 @@ func NewGatewayValidatingWebhook(cfg WebhookConfig) (*http.Server, error) { alwaysAccept, readGatewaysFromAllNamespaces, webhookNamespace, + kubeGatewayEnabled, ) mux := http.NewServeMux() @@ -165,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 { @@ -183,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) { @@ -441,6 +467,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 +506,17 @@ 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 wh.kubeGatewayEnabled + } + + // 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) 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..3fa426a5245 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() { @@ -52,7 +54,6 @@ var _ = Describe("ValidatingAdmissionWebhook", func() { validator: mv, } srv = httptest.NewServer(wh) - }) setMockFunctions := func() { @@ -110,6 +111,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) @@ -431,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 { diff --git a/projects/gateway/pkg/validation/validator.go b/projects/gateway/pkg/validation/validator.go index e0666913e8e..7c59d0f1339 100644 --- a/projects/gateway/pkg/validation/validator.go +++ b/projects/gateway/pkg/validation/validator.go @@ -13,8 +13,10 @@ import ( "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" + 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" @@ -245,10 +247,11 @@ type validationOutput struct { err error } -// 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. // @@ -283,34 +286,15 @@ 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) + validatingK8sGateway := isK8sGatewayProxy(opts.Resource) + if validatingK8sGateway { + proxies, errs = k8sgwvalidation.TranslateK8sGatewayProxies(ctx, snapshot, 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 @@ -337,6 +321,17 @@ 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. 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); err != nil { + errs = multierror.Append(errs, err) + } + continue + } + // Collect the reports returned by the glooValidator proxyReport := glooReports[0].ProxyReport proxyReports = append(proxyReports, proxyReport) @@ -355,7 +350,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. @@ -378,6 +372,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 { @@ -680,7 +729,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 } @@ -692,7 +749,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/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 { diff --git a/projects/gateway2/controller/start.go b/projects/gateway2/controller/start.go index 64b1eabf68d..bf00842c4a0 100644 --- a/projects/gateway2/controller/start.go +++ b/projects/gateway2/controller/start.go @@ -19,7 +19,6 @@ 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/gloo/projects/gloo/pkg/translator" "github.com/solo-io/solo-kit/pkg/api/v2/reporter" ) @@ -98,10 +97,6 @@ func Start(ctx context.Context, cfg StartConfig) error { // 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{ @@ -121,7 +116,6 @@ func Start(ctx context.Context, cfg StartConfig) error { proxySyncer := proxy_syncer.NewProxySyncer( wellknown.GatewayControllerName, cfg.Opts.WriteNamespace, - glooTranslator, inputChannels, mgr, k8sGwExtensions, diff --git a/projects/gateway2/install.sh b/projects/gateway2/install.sh index 20785019396..1d05e351ff7 100755 --- a/projects/gateway2/install.sh +++ b/projects/gateway2/install.sh @@ -3,4 +3,9 @@ set -eux ./ci/kind/setup-kind.sh -helm upgrade -i -n gloo-system gloo ./_test/gloo-1.0.0-ci1.tgz --create-namespace --set kubeGateway.enabled=true +helm upgrade --install --create-namespace \ + --namespace gloo-system gloo \ + ./_test/gloo-1.0.0-ci1.tgz \ + --set kubeGateway.enabled=true \ + --set discovery.enabled=false \ + --set gateway.validation.alwaysAcceptResources=false diff --git a/projects/gateway2/proxy_syncer/proxy_syncer.go b/projects/gateway2/proxy_syncer/proxy_syncer.go index 2dd1e36a9f1..a409c83d51d 100644 --- a/projects/gateway2/proxy_syncer/proxy_syncer.go +++ b/projects/gateway2/proxy_syncer/proxy_syncer.go @@ -19,7 +19,6 @@ import ( gwplugins "github.com/solo-io/gloo/projects/gateway2/translator/plugins" "github.com/solo-io/gloo/projects/gateway2/translator/plugins/registry" gloo_solo_io "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" - "github.com/solo-io/gloo/projects/gloo/pkg/translator" "github.com/solo-io/gloo/projects/gloo/pkg/utils" ) @@ -29,7 +28,6 @@ type QueueStatusForProxiesFn func(proxies gloo_solo_io.ProxyList, pluginRegistry // ProxySyncer is responsible for translating Kubernetes Gateway CRs into Gloo Proxies // and syncing the proxyClient with the newly translated proxies. type ProxySyncer struct { - translator translator.Translator controllerName string writeNamespace string @@ -72,7 +70,6 @@ func NewGatewayInputChannels() *GatewayInputChannels { // we reconcile gateway in the gateway controller. The `secretEvent` is kicked when a secret is created, updated, func NewProxySyncer( controllerName, writeNamespace string, - translator translator.Translator, inputs *GatewayInputChannels, mgr manager.Manager, k8sGwExtensions extensions.K8sGatewayExtensions, @@ -82,7 +79,6 @@ func NewProxySyncer( return &ProxySyncer{ controllerName: controllerName, writeNamespace: writeNamespace, - translator: translator, inputs: inputs, mgr: mgr, k8sGwExtensions: k8sGwExtensions, diff --git a/projects/gateway2/validation/validator.go b/projects/gateway2/validation/validator.go new file mode 100644 index 00000000000..2aad543ef74 --- /dev/null +++ b/projects/gateway2/validation/validator.go @@ -0,0 +1,104 @@ +package validation + +import ( + "context" + + sologatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" + 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" +) + +// 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{ + Static: &static.UpstreamSpec{ + Hosts: []*static.Host{ + {Addr: "solo.io", Port: 80}, + }, + }, + } + snap.UpsertToResourceList(us) + + routes := []*gloov1.Route{{ + Name: "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{ + "vhost": { + Name: "vhost", + Domains: []string{"*"}, + Routes: routes, + }, + }, + }, + HttpFilterChains: []*gloov1.AggregateListener_HttpFilterChain{{ + HttpOptionsRef: "opts1", + VirtualHostRefs: []string{"vhost"}, + }}, + }, + }, + } + + 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().GetVirtualHosts()["vhost"].Options = policy.GetOptions() + } + + 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, +) error { + var errs error + + for _, report := range reports { + proxyResReport := report.ResourceReports[proxy] + if proxyResReport.Errors != nil { + return proxyResReport.Errors + } + } + + return errs +} diff --git a/projects/gateway2/validation/validator_suite_test.go b/projects/gateway2/validation/validator_suite_test.go new file mode 100644 index 00000000000..9200d664ebe --- /dev/null +++ b/projects/gateway2/validation/validator_suite_test.go @@ -0,0 +1,16 @@ +package validation_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "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 new file mode 100644 index 00000000000..8ca1a0f9313 --- /dev/null +++ b/projects/gateway2/validation/validator_test.go @@ -0,0 +1,231 @@ +package validation_test + +import ( + "context" + + "github.com/golang/mock/gomock" + . "github.com/onsi/ginkgo/v2" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/wrapperspb" + "k8s.io/client-go/kubernetes/fake" + + . "github.com/onsi/gomega" + + sologatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1" + "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" + "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" + 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/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" + + gwv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +var _ = Describe("Kube Gateway API Policy Validation Helper", func() { + var ( + ctx context.Context + cancel context.CancelFunc + + ctrl *gomock.Controller + settings *v1.Settings + vc gloovalidation.ValidatorConfig + ) + + BeforeEach(func() { + ctx, cancel = context.WithCancel(context.Background()) + + resourceClientFactory := &factory.MemoryResourceClientFactory{ + Cache: memory.NewInMemoryResourceCache(), + } + + 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) + 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, + }, + } + }) + + AfterEach(func() { + cancel() + }) + + It("validates and rejects a bad RouteOption", func() { + gv := gloovalidation.NewValidator(vc) + + rtOpt := routeOptWithBadConfig() + params := plugins.Params{ + Ctx: ctx, + Snapshot: samples.SimpleGlooSnapshot("gloo-system"), + } + 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]) + 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)) + r := rpt[0] + proxyResourceReport := r.ResourceReports[proxies[0]] + Expect(proxyResourceReport.Errors.Error()).To(ContainSubstring(faultErrorMsg)) + }) + + It("validates and accepts a good RouteOption", func() { + gv := gloovalidation.NewValidator(vc) + + rtOpt := routeOptWithGoodConfig() + params := plugins.Params{ + Ctx: ctx, + Snapshot: samples.SimpleGlooSnapshot("gloo-system"), + } + 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]) + Expect(err).NotTo(HaveOccurred()) + r := rpt[0] + proxyResourceReport := r.ResourceReports[proxies[0]] + Expect(proxyResourceReport.Errors).NotTo(HaveOccurred()) + }) + + It("validates and a rejects a bad VirtualHostOption", func() { + gv := gloovalidation.NewValidator(vc) + + params := plugins.Params{ + Ctx: ctx, + Snapshot: samples.SimpleGlooSnapshot("gloo-system"), + } + vhost := vHostOptWithBadConfig() + proxies, _ := validation.TranslateK8sGatewayProxies(ctx, params.Snapshot, vhost) + gv.Sync(ctx, params.Snapshot) + rpt, err := gv.ValidateGloo(ctx, proxies[0], vhost, false) + Expect(err).NotTo(HaveOccurred()) + err = validation.GetSimpleErrorFromGlooValidation(rpt, proxies[0]) + Expect(err).To(HaveOccurred()) + 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]] + Expect(proxyResourceReport.Errors.Error()).To(ContainSubstring(bufferErrorMsg)) + }) + + It("validates and accepts a good VirtualHostOption", func() { + gv := gloovalidation.NewValidator(vc) + + params := plugins.Params{ + Ctx: ctx, + Snapshot: samples.SimpleGlooSnapshot("gloo-system"), + } + vhost := vHostOptWithGoodConfig() + proxies, _ := validation.TranslateK8sGatewayProxies(ctx, params.Snapshot, vhost) + gv.Sync(ctx, params.Snapshot) + rpt, err := gv.ValidateGloo(ctx, proxies[0], vhost, false) + Expect(err).NotTo(HaveOccurred()) + err = validation.GetSimpleErrorFromGlooValidation(rpt, proxies[0]) + Expect(err).ToNot(HaveOccurred()) + r := rpt[0] + proxyResourceReport := r.ResourceReports[proxies[0]] + Expect(proxyResourceReport.Errors).NotTo(HaveOccurred()) + }) +}) + +func vHostOptWithBadConfig() *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 vHostOptWithGoodConfig() *sologatewayv1.VirtualHostOption { + vHostOpt := proto.Clone(vHostOptWithBadConfig()).(*sologatewayv1.VirtualHostOption) + vHostOpt.GetOptions().GetBufferPerRoute().GetBuffer().MaxRequestBytes = wrapperspb.UInt32(1024) + return vHostOpt +} + +func routeOptWithBadConfig() *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, + }, + }, + }, + } +} + +func routeOptWithGoodConfig() *sologatewayv1.RouteOption { + rtOpt := proto.Clone(routeOptWithBadConfig()).(*sologatewayv1.RouteOption) + rtOpt.GetOptions().GetFaults().GetAbort().HttpStatus = 500 + return rtOpt +} 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). diff --git a/projects/gloo/pkg/syncer/setup/setup_syncer.go b/projects/gloo/pkg/syncer/setup/setup_syncer.go index 636fbefcbb0..f6599959675 100644 --- a/projects/gloo/pkg/syncer/setup/setup_syncer.go +++ b/projects/gloo/pkg/syncer/setup/setup_syncer.go @@ -904,8 +904,8 @@ func RunGlooWithExtensions(opts bootstrap.Opts, extensions Extensions) error { authConfigClient, routeOptionClient, virtualHostOptionClient, + statusClient, ) - } translationSync := syncer.NewTranslatorSyncer( @@ -984,6 +984,7 @@ func RunGlooWithExtensions(opts bootstrap.Opts, extensions Extensions) error { gwOpts.Validation.AlwaysAcceptResources, gwOpts.ReadGatewaysFromAllNamespaces, gwOpts.GlooNamespace, + opts.GlooGateway.EnableK8sGatewayController, // controls validation of KubeGateway policies (e.g. RouteOption, VirtualHostOption) ), ) if err != nil { diff --git a/projects/gloo/pkg/syncer/setup/start_func.go b/projects/gloo/pkg/syncer/setup/start_func.go index 14daae65bda..05a395e10c0 100644 --- a/projects/gloo/pkg/syncer/setup/start_func.go +++ b/projects/gloo/pkg/syncer/setup/start_func.go @@ -6,9 +6,9 @@ import ( "golang.org/x/sync/errgroup" "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" - "github.com/solo-io/gloo/pkg/utils/statusutils" 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/proxy_syncer" @@ -58,6 +58,7 @@ func K8sGatewayControllerStartFunc( 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 { @@ -67,9 +68,7 @@ func K8sGatewayControllerStartFunc( opts.ProxyDebugServer.Server.RegisterProxyReader(debug.K8sGatewayTranslation, proxyClient) } - statusClient := statusutils.GetStatusClientForNamespace(opts.StatusReporterNamespace) statusReporter := reporter.NewReporter(defaults.KubeGatewayReporter, statusClient, routeOptionClient.BaseClient(), vhOptionClient.BaseClient()) - return controller.Start(ctx, controller.StartConfig{ ExtensionsFactory: extensions.K8sGatewayExtensionsFactory, GlooPluginRegistryFactory: extensions.PluginRegistryFactory, diff --git a/test/kubernetes/e2e/features/route_options/suite.go b/test/kubernetes/e2e/features/route_options/suite.go index 9d090e06c83..db0af6ae9e0 100644 --- a/test/kubernetes/e2e/features/route_options/suite.go +++ b/test/kubernetes/e2e/features/route_options/suite.go @@ -33,7 +33,10 @@ type testingSuite struct { manifests map[string][]string } -func NewTestingSuite(ctx context.Context, testInst *e2e.TestInstallation) suite.TestingSuite { +func NewTestingSuite( + ctx context.Context, + testInst *e2e.TestInstallation, +) suite.TestingSuite { return &testingSuite{ 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 be7e9b271e0..ac8f0680758 100644 --- a/test/kubernetes/e2e/features/virtualhost_options/suite.go +++ b/test/kubernetes/e2e/features/virtualhost_options/suite.go @@ -32,7 +32,10 @@ type testingSuite struct { manifests map[string][]string } -func NewTestingSuite(ctx context.Context, testInst *e2e.TestInstallation) suite.TestingSuite { +func NewTestingSuite( + ctx context.Context, + testInst *e2e.TestInstallation, +) suite.TestingSuite { return &testingSuite{ ctx: ctx, testInstallation: testInst, @@ -74,8 +77,8 @@ 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) + output, err := s.testInstallation.Actions.Kubectl().ApplyFileWithOutput(s.ctx, manifest) + s.testInstallation.Assertions.ExpectObjectAdmitted(manifest, err, output, "Validating *v1.VirtualHostOption failed") } } @@ -86,8 +89,8 @@ func (s *testingSuite) AfterTest(suiteName, testName string) { } for _, manifest := range manifests { - 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) } } @@ -111,12 +114,20 @@ 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.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( + 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/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 e836e607904..ee8c7c565ec 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/tests/k8s_gw_no_validation_test.go b/test/kubernetes/e2e/tests/k8s_gw_no_validation_test.go new file mode 100644 index 00000000000..649239c6e8c --- /dev/null +++ b/test/kubernetes/e2e/tests/k8s_gw_no_validation_test.go @@ -0,0 +1,63 @@ +package tests_test + +import ( + "context" + "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() + testInstallation := e2e.CreateTestInstallation( + t, + &gloogateway.Context{ + 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) + + // 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() + }) + }) + + // 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)) + }) + + t.Run("VirtualHostOptions", func(t *testing.T) { + suite.Run(t, virtualhost_options.NewTestingSuite(ctx, testInstallation)) + }) + + t.Run("PortRouting", func(t *testing.T) { + suite.Run(t, port_routing.NewTestingSuite(ctx, testInstallation)) + + }) +} diff --git a/test/kubernetes/e2e/tests/k8s_gw_test.go b/test/kubernetes/e2e/tests/k8s_gw_test.go index 76bbb82bc5a..cebd38e135f 100644 --- a/test/kubernetes/e2e/tests/k8s_gw_test.go +++ b/test/kubernetes/e2e/tests/k8s_gw_test.go @@ -27,8 +27,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, }, ) diff --git a/test/kubernetes/e2e/tests/manifests/k8s-gateway-no-webhook-validation-test-helm.yaml b/test/kubernetes/e2e/tests/manifests/k8s-gateway-no-webhook-validation-test-helm.yaml new file mode 100644 index 00000000000..a2ad326f895 --- /dev/null +++ b/test/kubernetes/e2e/tests/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/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 912522854df..84c4e6bd0c7 100644 --- a/test/kubernetes/testutils/assertions/objects.go +++ b/test/kubernetes/testutils/assertions/objects.go @@ -3,11 +3,14 @@ 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" ) @@ -42,3 +45,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/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" 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 }