From 1bed38058d42bf163538321fbcc88bf4313d9c86 Mon Sep 17 00:00:00 2001 From: Shashank Ram <21697719+shashankram@users.noreply.github.com> Date: Mon, 16 Dec 2024 08:58:30 -0800 Subject: [PATCH] gateway2/delegation: enable inherited policy overrides (#10470) Signed-off-by: Shashank Ram --- changelog/v1.19.0-beta3/fix-sp-7315.yaml | 20 +++ .../translator/gateway_translator_test.go | 3 + .../routeoptions/route_options_plugin.go | 79 ++++++++- .../routeoptions/route_options_plugin_test.go | 160 +++++++++++++++++- .../gateway2/translator/routeutils/utils.go | 17 +- ...ions_inheritance_child_override_allow.yaml | 106 ++++++++++++ ...ulti_level_inheritance_override_allow.yaml | 147 ++++++++++++++++ ...ti_level_inheritance_override_partial.yaml | 146 ++++++++++++++++ ...ions_inheritance_child_override_allow.yaml | 71 ++++++++ ...ulti_level_inheritance_override_allow.yaml | 88 ++++++++++ ...ti_level_inheritance_override_partial.yaml | 88 ++++++++++ projects/gloo/pkg/utils/merge.go | 79 +++++++++ 12 files changed, 990 insertions(+), 14 deletions(-) create mode 100644 changelog/v1.19.0-beta3/fix-sp-7315.yaml create mode 100644 projects/gateway2/translator/testutils/inputs/delegation/route_options_inheritance_child_override_allow.yaml create mode 100644 projects/gateway2/translator/testutils/inputs/delegation/route_options_multi_level_inheritance_override_allow.yaml create mode 100644 projects/gateway2/translator/testutils/inputs/delegation/route_options_multi_level_inheritance_override_partial.yaml create mode 100644 projects/gateway2/translator/testutils/outputs/delegation/route_options_inheritance_child_override_allow.yaml create mode 100644 projects/gateway2/translator/testutils/outputs/delegation/route_options_multi_level_inheritance_override_allow.yaml create mode 100644 projects/gateway2/translator/testutils/outputs/delegation/route_options_multi_level_inheritance_override_partial.yaml diff --git a/changelog/v1.19.0-beta3/fix-sp-7315.yaml b/changelog/v1.19.0-beta3/fix-sp-7315.yaml new file mode 100644 index 00000000000..768156feb0d --- /dev/null +++ b/changelog/v1.19.0-beta3/fix-sp-7315.yaml @@ -0,0 +1,20 @@ +changelog: + - type: FIX + issueLink: https://github.com/solo-io/solo-projects/issues/7315 + resolvesIssue: false + description: | + gateway2/delegation: enable inherited policy overrides + + Adds the ability to override inherited policy fields when + explicitly permitted by a parent route using the annotation + delegation.gateway.solo.io/enable-policy-overrides. + It supports a wildcard value "*" or a comma separated list + of field names such as "faults,timeouts,retries,headermanipulation". + + Functionally, a child RouteOption may only override the RouteOptions + derived from its parent if the above annotation exists on the parent + route. This is required to make the override behavior safe to use. + + Testing done: + - Translator tests for the new scenarios. + diff --git a/projects/gateway2/translator/gateway_translator_test.go b/projects/gateway2/translator/gateway_translator_test.go index 765152f3561..0bb33a6cc57 100644 --- a/projects/gateway2/translator/gateway_translator_test.go +++ b/projects/gateway2/translator/gateway_translator_test.go @@ -326,4 +326,7 @@ var _ = DescribeTable("Route Delegation translator", Entry("Child route matcher does not match parent", "bug-6621.yaml"), // https://github.com/k8sgateway/k8sgateway/issues/10379 Entry("Multi-level multiple parents delegation", "bug-10379.yaml"), + Entry("RouteOptions prefer child override when allowed", "route_options_inheritance_child_override_allow.yaml"), + Entry("RouteOptions multi level inheritance with child override when allowed", "route_options_multi_level_inheritance_override_allow.yaml"), + Entry("RouteOptions multi level inheritance with partial child override", "route_options_multi_level_inheritance_override_partial.yaml"), ) diff --git a/projects/gateway2/translator/plugins/routeoptions/route_options_plugin.go b/projects/gateway2/translator/plugins/routeoptions/route_options_plugin.go index 8bb543e5f41..03eea5fb2be 100644 --- a/projects/gateway2/translator/plugins/routeoptions/route_options_plugin.go +++ b/projects/gateway2/translator/plugins/routeoptions/route_options_plugin.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strings" "github.com/hashicorp/go-multierror" "github.com/rotisserie/eris" @@ -15,6 +16,7 @@ import ( "istio.io/istio/pkg/kube/krt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" gwv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -31,6 +33,15 @@ import ( glooutils "github.com/solo-io/gloo/projects/gloo/pkg/utils" ) +const ( + // policyOverrideAnnotation can be set by parent routes to allow child routes to override + // all (wildcard *) or specific fields (comma separated field names) in RouteOptions inherited from the parent route. + policyOverrideAnnotation = "delegation.gateway.solo.io/enable-policy-overrides" + + // wildcardField is used to enable overriding all fields in RouteOptions inherited from the parent route. + wildcardField = "*" +) + var ( _ plugins.RoutePlugin = &plugin{} _ plugins.StatusPlugin = &plugin{} @@ -83,7 +94,7 @@ func (p *plugin) ApplyRoutePlugin( routeCtx *plugins.RouteContext, outputRoute *gloov1.Route, ) error { - // check for RouteOptions applied to full Route + // check for RouteOptions applied to the given routeCtx routeOptions, _, sources, err := p.handleAttachment(ctx, routeCtx) if err != nil { return err @@ -92,22 +103,59 @@ func (p *plugin) ApplyRoutePlugin( return nil } - // If the route already has options set, we should override them. - // This is important because for delegated routes, the plugin will - // be invoked on the child routes multiple times for each parent route - // that may override them. - merged, usedExistingSources := glooutils.ShallowMergeRouteOptions(routeOptions, outputRoute.GetOptions()) + merged, OptionsMergeResult := mergeOptionsForRoute(ctx, routeCtx.HTTPRoute, routeOptions, outputRoute.GetOptions()) + if OptionsMergeResult == glooutils.OptionsMergedNone { + // No existing options merged into 'sources', so set the 'sources' on the outputRoute + routeutils.SetRouteSources(outputRoute, sources) + } else if OptionsMergeResult == glooutils.OptionsMergedPartial { + // Some existing options merged into 'sources', so append the 'sources' on the outputRoute + routeutils.AppendRouteSources(outputRoute, sources) + } // In case OptionsMergedFull, the correct sources are already set on the outputRoute + + // Set the merged RouteOptions on the outputRoute outputRoute.Options = merged - // Track the RouteOption policy sources that are used so we can report status on it - routeutils.AppendSourceToRoute(outputRoute, sources, usedExistingSources) // Track that we used this RouteOption is our status cache // we do this so we can persist status later for all attached RouteOptions - p.trackAcceptedRouteOptions(sources) + p.trackAcceptedRouteOptions(outputRoute.GetMetadataStatic().GetSources()) return nil } +func mergeOptionsForRoute( + ctx context.Context, + route *gwv1.HTTPRoute, + dst, src *gloov1.RouteOptions, +) (*gloov1.RouteOptions, glooutils.OptionsMergeResult) { + // By default, lower priority options cannot override higher priority ones + // and can only augment them during a merge such that fields unset in the higher + // priority options can be merged in from the lower priority options. + // In the case of delegated routes, a parent route can enable child routes to override + // all (wildcard *) or specific fields using the policyOverrideAnnotation. + fieldsAllowedToOverride := sets.New[string]() + + // If the route already has options set, we should override/augment them. + // This is important because for delegated routes, the plugin will + // be invoked on the child routes multiple times for each parent route + // that may override/augment them. + // + // By default, parent options (routeOptions) are preferred, unless the parent explicitly + // enabled child routes (outputRoute.Options) to override parent options. + fieldsStr, delegatedPolicyOverride := route.Annotations[policyOverrideAnnotation] + if delegatedPolicyOverride { + delegatedFieldsToOverride := parseDelegationFieldOverrides(fieldsStr) + if delegatedFieldsToOverride.Len() == 0 { + // Invalid annotation value, so log an error but enforce the default behavior of preferring the parent options. + contextutils.LoggerFrom(ctx).Errorf("invalid value %q for annotation %s on route %s; must be %s or a comma-separated list of field names", + fieldsStr, policyOverrideAnnotation, client.ObjectKeyFromObject(route), wildcardField) + } else { + fieldsAllowedToOverride = delegatedFieldsToOverride + } + } + + return glooutils.MergeRouteOptionsWithOverrides(dst, src, fieldsAllowedToOverride) +} + func (p *plugin) InitStatusPlugin(ctx context.Context, statusCtx *plugins.StatusContext) error { for _, proxyWithReport := range statusCtx.ProxiesWithReports { // now that we translate proxies one by one, we can't assume ApplyRoutePlugin is called before ApplyStatusPlugin for all proxies @@ -300,3 +348,16 @@ func extractRouteOptionSourceKeys(routeErr *validation.RouteReport_Error) (types return types.NamespacedName{}, false } + +func parseDelegationFieldOverrides(val string) sets.Set[string] { + if val == wildcardField { + return sets.New(wildcardField) + } + + set := sets.New[string]() + parts := strings.Split(val, ",") + for _, part := range parts { + set.Insert(strings.ToLower(strings.TrimSpace(part))) + } + return set +} diff --git a/projects/gateway2/translator/plugins/routeoptions/route_options_plugin_test.go b/projects/gateway2/translator/plugins/routeoptions/route_options_plugin_test.go index 7a6d746cc59..a0497cb53e4 100644 --- a/projects/gateway2/translator/plugins/routeoptions/route_options_plugin_test.go +++ b/projects/gateway2/translator/plugins/routeoptions/route_options_plugin_test.go @@ -27,6 +27,7 @@ import ( 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/defaults" + glooutils "github.com/solo-io/gloo/projects/gloo/pkg/utils" 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" @@ -354,7 +355,6 @@ var _ = Describe("RouteOptionsPlugin", func() { err := plugin.ApplyStatusPlugin(ctx, statusCtx) Expect(err).To(MatchError(ContainSubstring(ReadingRouteOptionErrStr))) }) - }) }) @@ -734,6 +734,164 @@ var _ = Describe("RouteOptionsPlugin", func() { }) }) +var _ = DescribeTable("mergeOptionsForRoute", + func(route *gwv1.HTTPRoute, dst, src *v1.RouteOptions, expectedOptions *v1.RouteOptions, expectedResult glooutils.OptionsMergeResult) { + mergedOptions, result := mergeOptionsForRoute(context.TODO(), route, dst, src) + Expect(proto.Equal(mergedOptions, expectedOptions)).To(BeTrue()) + Expect(result).To(Equal(expectedResult)) + }, + Entry("prefer dst options by default", + &gwv1.HTTPRoute{}, + &v1.RouteOptions{ + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 500, + }, + }, + }, + &v1.RouteOptions{ + PrefixRewrite: &wrapperspb.StringValue{Value: "/prefix"}, + // Faults will be ignored because it is set in dst + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 100, + }, + }, + }, + &v1.RouteOptions{ + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 500, + }, + }, + PrefixRewrite: &wrapperspb.StringValue{Value: "/prefix"}, + }, + glooutils.OptionsMergedPartial, + ), + Entry("override dst options with annotation: full override", + &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{policyOverrideAnnotation: "*"}, + }, + }, + &v1.RouteOptions{ + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 500, + }, + }, + }, + &v1.RouteOptions{ + PrefixRewrite: &wrapperspb.StringValue{Value: "/prefix"}, + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 100, + }, + }, + }, + &v1.RouteOptions{ + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 100, + }, + }, + PrefixRewrite: &wrapperspb.StringValue{Value: "/prefix"}, + }, + glooutils.OptionsMergedFull, + ), + Entry("override dst options with annotation: partial override", + &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{policyOverrideAnnotation: "*"}, + }, + }, + &v1.RouteOptions{ + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 500, + }, + }, + Timeout: durationpb.New(5 * time.Second), + }, + &v1.RouteOptions{ + PrefixRewrite: &wrapperspb.StringValue{Value: "/prefix"}, + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 100, + }, + }, + }, + &v1.RouteOptions{ + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 100, + }, + }, + PrefixRewrite: &wrapperspb.StringValue{Value: "/prefix"}, + Timeout: durationpb.New(5 * time.Second), + }, + glooutils.OptionsMergedPartial, + ), + Entry("override dst options with annotation: no override", + &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{policyOverrideAnnotation: "*"}, + }, + }, + &v1.RouteOptions{ + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 500, + }, + }, + }, + &v1.RouteOptions{}, + &v1.RouteOptions{ + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 500, + }, + }, + }, + glooutils.OptionsMergedNone, + ), + Entry("override dst options with annotation: specific fields", + &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{policyOverrideAnnotation: "faults,timeout"}, + }, + }, + &v1.RouteOptions{ + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 500, + }, + }, + Timeout: durationpb.New(5 * time.Second), + PrefixRewrite: &wrapperspb.StringValue{Value: "/dst"}, + }, + &v1.RouteOptions{ + PrefixRewrite: &wrapperspb.StringValue{Value: "/src"}, + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 100, + }, + }, + Timeout: durationpb.New(10 * time.Second), + }, + &v1.RouteOptions{ + Faults: &faultinjection.RouteFaults{ + Abort: &faultinjection.RouteAbort{ + HttpStatus: 100, + }, + }, + PrefixRewrite: &wrapperspb.StringValue{Value: "/dst"}, + Timeout: durationpb.New(10 * time.Second), + }, + glooutils.OptionsMergedFull, + ), +) + func routeOption() *solokubev1.RouteOption { return &solokubev1.RouteOption{ ObjectMeta: metav1.ObjectMeta{ diff --git a/projects/gateway2/translator/routeutils/utils.go b/projects/gateway2/translator/routeutils/utils.go index 64bd72657d1..39a51d3aea8 100644 --- a/projects/gateway2/translator/routeutils/utils.go +++ b/projects/gateway2/translator/routeutils/utils.go @@ -5,15 +5,12 @@ import ( v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" ) -func AppendSourceToRoute(route *v1.Route, newSources []*gloov1.SourceMetadata_SourceRef, preserveExisting bool) { +func AppendRouteSources(route *v1.Route, newSources []*gloov1.SourceMetadata_SourceRef) { meta := route.GetMetadataStatic() if meta == nil { meta = &gloov1.SourceMetadata{} } sources := meta.GetSources() - if !preserveExisting { - sources = nil - } sources = append(sources, newSources...) route.OpaqueMetadata = &gloov1.Route_MetadataStatic{ MetadataStatic: &gloov1.SourceMetadata{ @@ -21,3 +18,15 @@ func AppendSourceToRoute(route *v1.Route, newSources []*gloov1.SourceMetadata_So }, } } + +func SetRouteSources(route *v1.Route, sources []*gloov1.SourceMetadata_SourceRef) { + meta := route.GetMetadataStatic() + if meta == nil { + meta = &gloov1.SourceMetadata{} + } + route.OpaqueMetadata = &gloov1.Route_MetadataStatic{ + MetadataStatic: &gloov1.SourceMetadata{ + Sources: sources, + }, + } +} diff --git a/projects/gateway2/translator/testutils/inputs/delegation/route_options_inheritance_child_override_allow.yaml b/projects/gateway2/translator/testutils/inputs/delegation/route_options_inheritance_child_override_allow.yaml new file mode 100644 index 00000000000..9a9f4fdd48f --- /dev/null +++ b/projects/gateway2/translator/testutils/inputs/delegation/route_options_inheritance_child_override_allow.yaml @@ -0,0 +1,106 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: example-gateway + namespace: infra +spec: + gatewayClassName: example-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: example-route + namespace: infra + annotations: + delegation.gateway.solo.io/enable-policy-overrides: "*" +spec: + parentRefs: + - name: example-gateway + hostnames: + - "example.com" + rules: + - backendRefs: + - name: example-svc + port: 80 + - matches: + - path: + type: PathPrefix + value: /a + backendRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: "*" + namespace: a +--- +apiVersion: gateway.solo.io/v1 +kind: RouteOption +metadata: + name: example-opt + namespace: infra +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: example-route + options: + faults: + abort: + percentage: 100 + httpStatus: 418 +--- +apiVersion: v1 +kind: Service +metadata: + name: example-svc + namespace: infra +spec: + selector: + test: test + ports: + - protocol: TCP + port: 80 +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: route-a + namespace: a +spec: + rules: + - matches: + - path: + type: PathPrefix + value: /a/1 + backendRefs: + - name: svc-a + port: 8080 +--- +apiVersion: gateway.solo.io/v1 +kind: RouteOption +metadata: + name: route-a-opt + namespace: a +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: route-a + options: + faults: + abort: + percentage: 80 + httpStatus: 404 +--- +apiVersion: v1 +kind: Service +metadata: + name: svc-a + namespace: a +spec: + ports: + - protocol: TCP + port: 8080 diff --git a/projects/gateway2/translator/testutils/inputs/delegation/route_options_multi_level_inheritance_override_allow.yaml b/projects/gateway2/translator/testutils/inputs/delegation/route_options_multi_level_inheritance_override_allow.yaml new file mode 100644 index 00000000000..351cad5e19e --- /dev/null +++ b/projects/gateway2/translator/testutils/inputs/delegation/route_options_multi_level_inheritance_override_allow.yaml @@ -0,0 +1,147 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: example-gateway + namespace: infra +spec: + gatewayClassName: example-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: example-route + namespace: infra + annotations: + delegation.gateway.solo.io/enable-policy-overrides: "*" +spec: + parentRefs: + - name: example-gateway + hostnames: + - "example.com" + rules: + - backendRefs: + - name: example-svc + port: 80 + - matches: + - path: + type: PathPrefix + value: /a + backendRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: "*" + namespace: a-root +--- +apiVersion: gateway.solo.io/v1 +kind: RouteOption +metadata: + name: example-opt + namespace: infra +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: example-route + options: + faults: + abort: + percentage: 100 + httpStatus: 418 + cors: + exposeHeaders: + - example + allowOrigin: + - example +--- +apiVersion: v1 +kind: Service +metadata: + name: example-svc + namespace: infra +spec: + selector: + test: test + ports: + - protocol: TCP + port: 80 +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: route-a-root + namespace: a-root + annotations: + delegation.gateway.solo.io/enable-policy-overrides: "*" +spec: + rules: + - matches: + - path: + type: PathPrefix + value: /a/1 + backendRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: "*" + namespace: a +--- +apiVersion: gateway.solo.io/v1 +kind: RouteOption +metadata: + name: route-a-root-opt + namespace: a-root +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: route-a-root + options: + headerManipulation: + requestHeadersToRemove: ["foo"] +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: route-a + namespace: a +spec: + rules: + - matches: + - path: + type: PathPrefix + value: /a/1 + backendRefs: + - name: svc-a + port: 8080 +--- +apiVersion: gateway.solo.io/v1 +kind: RouteOption +metadata: + name: route-a-opt + namespace: a +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: route-a + options: + headerManipulation: + requestHeadersToRemove: ["override"] + cors: + exposeHeaders: + - foo + allowOrigin: + - baz +--- +apiVersion: v1 +kind: Service +metadata: + name: svc-a + namespace: a +spec: + ports: + - protocol: TCP + port: 8080 diff --git a/projects/gateway2/translator/testutils/inputs/delegation/route_options_multi_level_inheritance_override_partial.yaml b/projects/gateway2/translator/testutils/inputs/delegation/route_options_multi_level_inheritance_override_partial.yaml new file mode 100644 index 00000000000..badfc3480a2 --- /dev/null +++ b/projects/gateway2/translator/testutils/inputs/delegation/route_options_multi_level_inheritance_override_partial.yaml @@ -0,0 +1,146 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: example-gateway + namespace: infra +spec: + gatewayClassName: example-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: example-route + namespace: infra + # Policy override annotation is missing +spec: + parentRefs: + - name: example-gateway + hostnames: + - "example.com" + rules: + - backendRefs: + - name: example-svc + port: 80 + - matches: + - path: + type: PathPrefix + value: /a + backendRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: "*" + namespace: a-root +--- +apiVersion: gateway.solo.io/v1 +kind: RouteOption +metadata: + name: example-opt + namespace: infra +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: example-route + options: + faults: + abort: + percentage: 100 + httpStatus: 418 + cors: + exposeHeaders: + - example + allowOrigin: + - example +--- +apiVersion: v1 +kind: Service +metadata: + name: example-svc + namespace: infra +spec: + selector: + test: test + ports: + - protocol: TCP + port: 80 +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: route-a-root + namespace: a-root + annotations: + delegation.gateway.solo.io/enable-policy-overrides: "headerManipulation,faults" +spec: + rules: + - matches: + - path: + type: PathPrefix + value: /a/1 + backendRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: "*" + namespace: a +--- +apiVersion: gateway.solo.io/v1 +kind: RouteOption +metadata: + name: route-a-root-opt + namespace: a-root +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: route-a-root + options: + headerManipulation: + requestHeadersToRemove: ["foo"] +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: route-a + namespace: a +spec: + rules: + - matches: + - path: + type: PathPrefix + value: /a/1 + backendRefs: + - name: svc-a + port: 8080 +--- +apiVersion: gateway.solo.io/v1 +kind: RouteOption +metadata: + name: route-a-opt + namespace: a +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: route-a + options: + headerManipulation: + requestHeadersToRemove: ["override"] + cors: + exposeHeaders: + - foo + allowOrigin: + - baz +--- +apiVersion: v1 +kind: Service +metadata: + name: svc-a + namespace: a +spec: + ports: + - protocol: TCP + port: 8080 diff --git a/projects/gateway2/translator/testutils/outputs/delegation/route_options_inheritance_child_override_allow.yaml b/projects/gateway2/translator/testutils/outputs/delegation/route_options_inheritance_child_override_allow.yaml new file mode 100644 index 00000000000..752e125d49f --- /dev/null +++ b/projects/gateway2/translator/testutils/outputs/delegation/route_options_inheritance_child_override_allow.yaml @@ -0,0 +1,71 @@ +--- +listeners: +- aggregateListener: + httpFilterChains: + - matcher: {} + virtualHostRefs: + - http~example_com + httpResources: + virtualHosts: + http~example_com: + domains: + - example.com + name: http~example_com + routes: + - matchers: + - prefix: /a/1 + metadataStatic: + sources: + - resourceKind: RouteOption + resourceRef: + name: route-a-opt + namespace: a + options: + faults: + abort: + httpStatus: 404 + percentage: 80 + name: httproute-route-a-a-0-0 + routeAction: + single: + kube: + port: 8080 + ref: + name: svc-a + namespace: a + - matchers: + - prefix: / + metadataStatic: + sources: + - resourceKind: RouteOption + resourceRef: + name: example-opt + namespace: infra + options: + faults: + abort: + httpStatus: 418 + percentage: 100 + name: httproute-example-route-infra-0-0 + routeAction: + single: + kube: + port: 80 + ref: + name: example-svc + namespace: infra + bindAddress: '::' + bindPort: 8080 + name: http + metadataStatic: + sources: + - resourceKind: gateway.networking.k8s.io/Gateway + resourceRef: + name: http + namespace: infra +metadata: + labels: + created_by: gloo-kube-gateway-api + gateway_namespace: infra + name: infra-example-gateway + namespace: gloo-system diff --git a/projects/gateway2/translator/testutils/outputs/delegation/route_options_multi_level_inheritance_override_allow.yaml b/projects/gateway2/translator/testutils/outputs/delegation/route_options_multi_level_inheritance_override_allow.yaml new file mode 100644 index 00000000000..212e308c6fd --- /dev/null +++ b/projects/gateway2/translator/testutils/outputs/delegation/route_options_multi_level_inheritance_override_allow.yaml @@ -0,0 +1,88 @@ +--- +listeners: +- aggregateListener: + httpFilterChains: + - matcher: {} + virtualHostRefs: + - http~example_com + httpResources: + virtualHosts: + http~example_com: + domains: + - example.com + name: http~example_com + routes: + - matchers: + - prefix: /a/1 + metadataStatic: + sources: + - resourceKind: RouteOption + resourceRef: + name: route-a-opt + namespace: a + - resourceKind: RouteOption + resourceRef: + name: example-opt + namespace: infra + options: + cors: + allowOrigin: + - baz + exposeHeaders: + - foo + faults: + abort: + httpStatus: 418 + percentage: 100 + headerManipulation: + requestHeadersToRemove: + - override + name: httproute-route-a-a-0-0 + routeAction: + single: + kube: + port: 8080 + ref: + name: svc-a + namespace: a + - matchers: + - prefix: / + metadataStatic: + sources: + - resourceKind: RouteOption + resourceRef: + name: example-opt + namespace: infra + options: + cors: + allowOrigin: + - example + exposeHeaders: + - example + faults: + abort: + httpStatus: 418 + percentage: 100 + name: httproute-example-route-infra-0-0 + routeAction: + single: + kube: + port: 80 + ref: + name: example-svc + namespace: infra + bindAddress: '::' + bindPort: 8080 + name: http + metadataStatic: + sources: + - resourceKind: gateway.networking.k8s.io/Gateway + resourceRef: + name: http + namespace: infra +metadata: + labels: + created_by: gloo-kube-gateway-api + gateway_namespace: infra + name: infra-example-gateway + namespace: gloo-system diff --git a/projects/gateway2/translator/testutils/outputs/delegation/route_options_multi_level_inheritance_override_partial.yaml b/projects/gateway2/translator/testutils/outputs/delegation/route_options_multi_level_inheritance_override_partial.yaml new file mode 100644 index 00000000000..688c56132ad --- /dev/null +++ b/projects/gateway2/translator/testutils/outputs/delegation/route_options_multi_level_inheritance_override_partial.yaml @@ -0,0 +1,88 @@ +--- +listeners: +- aggregateListener: + httpFilterChains: + - matcher: {} + virtualHostRefs: + - http~example_com + httpResources: + virtualHosts: + http~example_com: + domains: + - example.com + name: http~example_com + routes: + - matchers: + - prefix: /a/1 + metadataStatic: + sources: + - resourceKind: RouteOption + resourceRef: + name: route-a-opt + namespace: a + - resourceKind: RouteOption + resourceRef: + name: example-opt + namespace: infra + options: + cors: + allowOrigin: + - example + exposeHeaders: + - example + faults: + abort: + httpStatus: 418 + percentage: 100 + headerManipulation: + requestHeadersToRemove: + - override + name: httproute-route-a-a-0-0 + routeAction: + single: + kube: + port: 8080 + ref: + name: svc-a + namespace: a + - matchers: + - prefix: / + metadataStatic: + sources: + - resourceKind: RouteOption + resourceRef: + name: example-opt + namespace: infra + options: + cors: + allowOrigin: + - example + exposeHeaders: + - example + faults: + abort: + httpStatus: 418 + percentage: 100 + name: httproute-example-route-infra-0-0 + routeAction: + single: + kube: + port: 80 + ref: + name: example-svc + namespace: infra + bindAddress: '::' + bindPort: 8080 + name: http + metadataStatic: + sources: + - resourceKind: gateway.networking.k8s.io/Gateway + resourceRef: + name: http + namespace: infra +metadata: + labels: + created_by: gloo-kube-gateway-api + gateway_namespace: infra + name: infra-example-gateway + namespace: gloo-system diff --git a/projects/gloo/pkg/utils/merge.go b/projects/gloo/pkg/utils/merge.go index eac9c4a3ba9..6cbedab218d 100644 --- a/projects/gloo/pkg/utils/merge.go +++ b/projects/gloo/pkg/utils/merge.go @@ -2,10 +2,29 @@ package utils import ( "reflect" + "strings" + + "k8s.io/apimachinery/pkg/util/sets" v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" ) +const wildcardField = "*" + +// OptionsMergeResult is an enum indicating the result of the merge of options from src to dst. +type OptionsMergeResult int + +const ( + // OptionsMergedNone indicates that no fields were merged from src to dst. + OptionsMergedNone OptionsMergeResult = iota + + // OptionsMergedPartial indicates that some but not all fields were merged from src to dst. + OptionsMergedPartial + + // OptionsMergedFull indicates that all fields set in dst were overwritten by src. + OptionsMergedFull +) + // ShallowMerge sets dst to the value of src, if src is non-zero and dst is zero-valued or overwrite=true. // It returns a boolean indicating whether src overwrote dst. func ShallowMerge(dst, src reflect.Value, overwrite bool) bool { @@ -98,3 +117,63 @@ func ShallowMergeVirtualHostOptions(dst, src *v1.VirtualHostOptions) (*v1.Virtua return dst, overwrote } + +// MergeRouteOptionsWithOverrides merges the top-level fields of src that are allowed to be merged into dst. +// allowedOverrides is a set of field names in lowercase that are allowed to be merged, and may contain a wildcard field "*" +// to allow all fields to be merged. +// When allowedOverrides is empty, only fields unset in dst and set in src will be merged into dst. +// +// It returns an Enum indicating the result of the merge. +func MergeRouteOptionsWithOverrides(dst, src *v1.RouteOptions, allowedOverrides sets.Set[string]) (*v1.RouteOptions, OptionsMergeResult) { + if src == nil { + return dst, OptionsMergedNone + } + + if dst == nil { + return src.Clone().(*v1.RouteOptions), OptionsMergedFull + } + + dstValue, srcValue := reflect.ValueOf(dst).Elem(), reflect.ValueOf(src).Elem() + + // By default, do not allow fields in src to overwrite fields in dst. + // If allowedOverrides is non-empty, enable overwrites for the allowed fields. + overwriteByDefault := false + if allowedOverrides.Len() > 0 { + overwriteByDefault = true + } + + var srcFieldsUsed int + var dstFieldsSet int + var dstFieldsOverwritten int + for i := range dstValue.NumField() { + dstField, srcField := dstValue.Field(i), srcValue.Field(i) + if overwriteByDefault && !(allowedOverrides.Has(wildcardField) || + allowedOverrides.Has(strings.ToLower(dstValue.Type().Field(i).Name))) { + continue + } + // NOTE: important to pre-compute this for use in the conditional below + // because dstFieldsOverwritten needs to be incremented based on the original value of dstField + // and not the state of the field after the merge + dstOverridable := dstField.CanSet() && !isEmptyValue(dstField) + if dstOverridable { + dstFieldsSet++ + } + if srcOverride := ShallowMerge(dstField, srcField, overwriteByDefault); srcOverride { + srcFieldsUsed++ + if dstOverridable { + dstFieldsOverwritten++ + } + } + } + + var overrideState OptionsMergeResult + if srcFieldsUsed == 0 { + overrideState = OptionsMergedNone + } else if dstFieldsSet == dstFieldsOverwritten { + overrideState = OptionsMergedFull + } else { + overrideState = OptionsMergedPartial + } + + return dst, overrideState +}