diff --git a/Taskfile.test-infra.yml b/Taskfile.test-infra.yml index f81cfc32..9e84e49b 100644 --- a/Taskfile.test-infra.yml +++ b/Taskfile.test-infra.yml @@ -1100,7 +1100,7 @@ tasks: vars: # Scenarios authored against this env's cluster names and downstream path. # Older fixtures targeting the previous cluster names are excluded here. - DEFAULT_SCENARIOS: extension-server-smoke waf-enforcement branded-error-page connector-offline-503 atomic-reject-isolation oidc-missing-secret-isolation networkservice-endpoints + DEFAULT_SCENARIOS: extension-server-smoke waf-enforcement waf-section-scoping branded-error-page connector-offline-503 atomic-reject-isolation oidc-missing-secret-isolation networkservice-endpoints # CLI_ARGS (after --) wins; else SCENARIOS env; else the default set. SELECTED: '{{.CLI_ARGS | default .SCENARIOS | default .DEFAULT_SCENARIOS}}' deps: diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 362209a7..8615607d 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -190,6 +190,26 @@ webhooks: resources: - networks sideEffects: None +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-networking-datumapis-com-v1alpha-trafficprotectionpolicy + failurePolicy: Fail + name: vtrafficprotectionpolicy-v1alpha.kb.io + rules: + - apiGroups: + - networking.datumapis.com + apiVersions: + - v1alpha + operations: + - CREATE + - UPDATE + resources: + - trafficprotectionpolicies + sideEffects: None - admissionReviewVersions: - v1 clientConfig: diff --git a/internal/controller/trafficprotectionpolicy_controller.go b/internal/controller/trafficprotectionpolicy_controller.go index 0e194d48..4f9a120b 100644 --- a/internal/controller/trafficprotectionpolicy_controller.go +++ b/internal/controller/trafficprotectionpolicy_controller.go @@ -60,6 +60,7 @@ const ( // +kubebuilder:rbac:groups=networking.datumapis.com,resources=trafficprotectionpolicies,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=networking.datumapis.com,resources=trafficprotectionpolicies/status,verbs=get;update;patch // +kubebuilder:rbac:groups=networking.datumapis.com,resources=trafficprotectionpolicies/finalizers,verbs=update +// +kubebuilder:rbac:groups=networking.datumapis.com,resources=httpproxies,verbs=get;list;watch // +kubebuilder:rbac:groups=events.k8s.io,resources=events,verbs=create;patch // +kubebuilder:rbac:groups=cert-manager.io,resources=certificates,verbs=get;list;watch @@ -106,7 +107,12 @@ func (r *TrafficProtectionPolicyReconciler) Reconcile(ctx context.Context, req N return ctrl.Result{}, err } - r.collectTrafficProtectionPolicyAttachments(ctx, trafficProtectionPolicies, upstreamGateways.Items, upstreamHTTPRoutes.Items) + var upstreamHTTPProxies networkingv1alpha.HTTPProxyList + if err := cl.GetClient().List(ctx, &upstreamHTTPProxies, client.InNamespace(req.Namespace)); err != nil { + return ctrl.Result{}, err + } + + r.collectTrafficProtectionPolicyAttachments(ctx, trafficProtectionPolicies, upstreamGateways.Items, upstreamHTTPRoutes.Items, upstreamHTTPProxies.Items) r.setProgrammedConditionsFromDownstream(ctx, downstreamNamespaceName, trafficProtectionPolicies) @@ -319,6 +325,7 @@ type policyGatewayTargetContext struct { type policyRouteTargetContext struct { *gatewayv1.HTTPRoute + proxyRuleNames sets.Set[string] attached bool attachedToRouteRules sets.Set[string] } @@ -328,6 +335,7 @@ func (r *TrafficProtectionPolicyReconciler) collectTrafficProtectionPolicyAttach trafficProtectionPolicies []*policyContext, upstreamGateways []gatewayv1.Gateway, upstreamHTTPRoutes []gatewayv1.HTTPRoute, + upstreamHTTPProxies []networkingv1alpha.HTTPProxy, ) []policyAttachment { logger := log.FromContext(ctx) @@ -341,10 +349,16 @@ func (r *TrafficProtectionPolicyReconciler) collectTrafficProtectionPolicyAttach routeMapSize := len(upstreamHTTPRoutes) gatewayMapSize := len(upstreamGateways) + proxiesByName := make(map[string]*networkingv1alpha.HTTPProxy, len(upstreamHTTPProxies)) + for i := range upstreamHTTPProxies { + proxiesByName[upstreamHTTPProxies[i].Name] = &upstreamHTTPProxies[i] + } + routeMap := make(map[client.ObjectKey]*policyRouteTargetContext, routeMapSize) for i, route := range upstreamHTTPRoutes { routeMap[client.ObjectKeyFromObject(&route)] = &policyRouteTargetContext{ - HTTPRoute: &upstreamHTTPRoutes[i], + HTTPRoute: &upstreamHTTPRoutes[i], + proxyRuleNames: httpProxyRuleNames(&upstreamHTTPRoutes[i], proxiesByName), } } @@ -478,7 +492,7 @@ func (r *TrafficProtectionPolicyReconciler) processTrafficProtectionPolicyForHTT } route.attached = true } else { - found := false + found := route.proxyRuleNames.Has(string(*targetRef.SectionName)) for _, r := range route.Spec.Rules { if r.Name != nil && *r.Name == *targetRef.SectionName { found = true @@ -813,6 +827,7 @@ func (r *TrafficProtectionPolicyReconciler) SetupWithManager(mgr mcmanager.Manag Watches(&networkingv1alpha.TrafficProtectionPolicy{}, EnqueueRequestForObjectNamespace). Watches(&gatewayv1.Gateway{}, EnqueueRequestForObjectNamespace). Watches(&gatewayv1.HTTPRoute{}, EnqueueRequestForObjectNamespace). + Watches(&networkingv1alpha.HTTPProxy{}, EnqueueRequestForObjectNamespace). WatchesRawSource(downstreamTPPSource). Named("trafficprotectionpolicy"). Complete(r) @@ -880,3 +895,21 @@ func (r NamespaceReconcileRequest) WithCluster(name multicluster.ClusterName) Na r.ClusterName = name return r } + +func httpProxyRuleNames(route *gatewayv1.HTTPRoute, proxiesByName map[string]*networkingv1alpha.HTTPProxy) sets.Set[string] { + proxyName := route.Name + if owner := metav1.GetControllerOf(route); owner != nil && owner.Kind == "HTTPProxy" { + proxyName = owner.Name + } + proxy, ok := proxiesByName[proxyName] + if !ok { + return nil + } + names := make(sets.Set[string]) + for _, rule := range proxy.Spec.Rules { + if rule.Name != nil { + names.Insert(string(*rule.Name)) + } + } + return names +} diff --git a/internal/controller/trafficprotectionpolicy_controller_test.go b/internal/controller/trafficprotectionpolicy_controller_test.go index 9985dd4d..4186781f 100644 --- a/internal/controller/trafficprotectionpolicy_controller_test.go +++ b/internal/controller/trafficprotectionpolicy_controller_test.go @@ -4,6 +4,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/uuid" @@ -44,6 +46,7 @@ func TestCollectTrafficProtectionPolicyAttachments(t *testing.T) { name string gateways []gatewayv1.Gateway httpRoutes []gatewayv1.HTTPRoute + httpProxies []networkingv1alpha.HTTPProxy trafficProtectionPolicies []networkingv1alpha.TrafficProtectionPolicy assert func(t *testContext, policyAttachments []policyAttachment) }{ @@ -362,6 +365,7 @@ func TestCollectTrafficProtectionPolicyAttachments(t *testing.T) { tppContexts, tt.gateways, tt.httpRoutes, + tt.httpProxies, ) testCtx := &testContext{ @@ -953,3 +957,100 @@ func newTrafficProtectionPolicy( return tpp } + +func TestTrafficProtectionPolicySectionResolvesAgainstHTTPProxy(t *testing.T) { + operatorConfig := config.NetworkServicesOperator{} + + newProxy := func(name string, ruleNames ...string) networkingv1alpha.HTTPProxy { + proxy := networkingv1alpha.HTTPProxy{ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: name}} + for _, n := range ruleNames { + proxy.Spec.Rules = append(proxy.Spec.Rules, networkingv1alpha.HTTPProxyRule{Name: ptr.To(gatewayv1.SectionName(n))}) + } + return proxy + } + routeOwnedBy := func(name, proxyName string) gatewayv1.HTTPRoute { + route := newHTTPRoute("default", name) + route.OwnerReferences = []metav1.OwnerReference{{ + APIVersion: networkingv1alpha.GroupVersion.String(), + Kind: "HTTPProxy", + Name: proxyName, + Controller: ptr.To(true), + }} + return *route + } + + tests := []struct { + name string + route gatewayv1.HTTPRoute + proxies []networkingv1alpha.HTTPProxy + section string + wantAccepted bool + }{ + { + name: "same-named proxy has the rule", + route: *newHTTPRoute("default", "alb"), + proxies: []networkingv1alpha.HTTPProxy{newProxy("alb", "exempt", "protected")}, + section: "protected", + wantAccepted: true, + }, + { + name: "owning proxy has the rule", + route: routeOwnedBy("route-1", "alb"), + proxies: []networkingv1alpha.HTTPProxy{newProxy("alb", "protected")}, + section: "protected", + wantAccepted: true, + }, + { + name: "proxy lacks the rule", + route: *newHTTPRoute("default", "alb"), + proxies: []networkingv1alpha.HTTPProxy{newProxy("alb", "exempt")}, + section: "protected", + }, + { + name: "no proxy", + route: *newHTTPRoute("default", "alb"), + section: "protected", + }, + { + name: "unrelated proxy is not consulted", + route: *newHTTPRoute("default", "alb"), + proxies: []networkingv1alpha.HTTPProxy{newProxy("other", "protected")}, + section: "protected", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + proxiesByName := map[string]*networkingv1alpha.HTTPProxy{} + for i := range tt.proxies { + proxiesByName[tt.proxies[i].Name] = &tt.proxies[i] + } + route := tt.route + routeMap := map[client.ObjectKey]*policyRouteTargetContext{ + client.ObjectKeyFromObject(&route): { + HTTPRoute: &route, + proxyRuleNames: httpProxyRuleNames(&route, proxiesByName), + }, + } + policy := &policyContext{TrafficProtectionPolicy: ptr.To(newTrafficProtectionPolicy("default", "tpp-1"))} + targetRef := gatewayv1alpha2.LocalPolicyTargetReferenceWithSectionName{ + LocalPolicyTargetReference: gatewayv1.LocalPolicyTargetReference{Kind: "HTTPRoute", Name: gatewayv1.ObjectName(route.Name)}, + SectionName: ptr.To(gatewayv1.SectionName(tt.section)), + } + + reconciler := &TrafficProtectionPolicyReconciler{Config: operatorConfig} + reconciler.processTrafficProtectionPolicyForHTTPRoute(t.Context(), routeMap, nil, nil, policy, targetRef) + + require.Len(t, policy.Status.Ancestors, 1) + cond := apimeta.FindStatusCondition(policy.Status.Ancestors[0].Conditions, string(gatewayv1.PolicyConditionAccepted)) + require.NotNil(t, cond) + if tt.wantAccepted { + assert.Equal(t, metav1.ConditionTrue, cond.Status) + assert.Equal(t, string(gatewayv1.PolicyReasonAccepted), cond.Reason) + } else { + assert.Equal(t, metav1.ConditionFalse, cond.Status) + assert.Equal(t, string(gatewayv1.PolicyReasonTargetNotFound), cond.Reason) + } + }) + } +} diff --git a/internal/extensionserver/cache/index.go b/internal/extensionserver/cache/index.go index fb0965ef..af8698ef 100644 --- a/internal/extensionserver/cache/index.go +++ b/internal/extensionserver/cache/index.go @@ -34,11 +34,12 @@ import ( // they are prepended to every policy's per-rule directive list. func BuildPolicyIndexFromClient(ctx context.Context, cl client.Client, baseDirectives []string) (*PolicyIndex, error) { idx := &PolicyIndex{ - DStoUS: make(map[string]string), - ProjectNames: make(map[string]string), - TPPs: make(map[string][]TPPInfo), - Connectors: make(map[ConnectorKey]ConnectorInfo), - VPCPods: make(map[VPCPodKey]VPCPodInfo), + DStoUS: make(map[string]string), + ProjectNames: make(map[string]string), + TPPs: make(map[string][]TPPInfo), + HTTPProxyRules: make(map[HTTPProxyKey][]string), + Connectors: make(map[ConnectorKey]ConnectorInfo), + VPCPods: make(map[VPCPodKey]VPCPodInfo), } if err := populateFromClient(ctx, cl, idx, baseDirectives); err != nil { return nil, err @@ -133,6 +134,13 @@ func populateFromClient(ctx context.Context, cl client.Client, idx *PolicyIndex, // Resolve the effective upstream namespace for the ConnectorKey, consistent // with TPP indexing above. In two-cluster replica HTTPProxies carry // UpstreamOwnerNamespaceLabel; in single-cluster fall back to proxy.Namespace. + ruleNames := make([]string, len(proxy.Spec.Rules)) + for i, rule := range proxy.Spec.Rules { + if rule.Name != nil { + ruleNames[i] = string(*rule.Name) + } + } + idx.HTTPProxyRules[HTTPProxyKey{Namespace: proxy.Namespace, Name: proxy.Name}] = ruleNames effectiveNS := proxy.Labels[downstreamclient.UpstreamOwnerNamespaceLabel] if effectiveNS == "" { effectiveNS = proxy.Namespace diff --git a/internal/extensionserver/cache/index_test.go b/internal/extensionserver/cache/index_test.go index bd5fcdd3..1d423486 100644 --- a/internal/extensionserver/cache/index_test.go +++ b/internal/extensionserver/cache/index_test.go @@ -15,6 +15,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client/fake" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" networkingv1alpha1 "go.datum.net/network-services-operator/api/v1alpha1" @@ -1664,3 +1665,40 @@ func TestBuildPolicyIndexFromClient_NetworkService_NoGalacticSliceLeavesTenantEm info := idx.VPCPods[VPCPodKey{UpstreamNS: upstreamNS, HTTPProxyName: "my-proxy", RuleIndex: 0}] assert.Empty(t, info.TenantID) } + +func TestBuildPolicyIndexFromClient_HTTPProxyRuleNames(t *testing.T) { + scheme := indexTestScheme(t) + name := func(s string) *gatewayv1.SectionName { return (*gatewayv1.SectionName)(&s) } + + tests := []struct { + name string + rules []networkingv1alpha.HTTPProxyRule + want []string + }{ + {name: "no rules", rules: nil, want: []string{}}, + { + name: "all named", + rules: []networkingv1alpha.HTTPProxyRule{{Name: name("exempt")}, {Name: name("protected")}}, + want: []string{"exempt", "protected"}, + }, + { + name: "unnamed rule keeps its position", + rules: []networkingv1alpha.HTTPProxyRule{{}, {Name: name("second")}}, + want: []string{"", "second"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + proxy := &networkingv1alpha.HTTPProxy{ + ObjectMeta: metav1.ObjectMeta{Name: "alb", Namespace: "ns-abc"}, + Spec: networkingv1alpha.HTTPProxySpec{Rules: tt.rules}, + } + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(proxy).Build() + + idx, err := BuildPolicyIndexFromClient(context.Background(), cl, nil) + require.NoError(t, err) + assert.Equal(t, tt.want, idx.HTTPProxyRules[HTTPProxyKey{Namespace: "ns-abc", Name: "alb"}]) + assert.NotContains(t, idx.HTTPProxyRules, HTTPProxyKey{Namespace: "other", Name: "alb"}) + }) + } +} diff --git a/internal/extensionserver/cache/types.go b/internal/extensionserver/cache/types.go index 38fed343..c679c387 100644 --- a/internal/extensionserver/cache/types.go +++ b/internal/extensionserver/cache/types.go @@ -53,6 +53,12 @@ type PolicyIndex struct { // namespaces are not (they are commonly all "default"). TPPs map[string][]TPPInfo + // HTTPProxyRules maps (downstream replica namespace, httpProxyName) to the + // rule names of that HTTPProxy, indexed by rule position. Entries are empty + // strings for unnamed rules. Envoy Gateway numbers HTTPRoute rules in the + // same order, so the position is the rule index in route and cluster names. + HTTPProxyRules map[HTTPProxyKey][]string + // Connectors maps (upstreamNS, httpProxyName, ruleIndex) to ConnectorInfo. // Only populated for HTTPProxy rules that have a Connector backend. // Accumulated across all engaged clusters. @@ -64,6 +70,11 @@ type PolicyIndex struct { VPCPods map[VPCPodKey]VPCPodInfo } +type HTTPProxyKey struct { + Namespace string + Name string +} + // TPPInfo holds the fields of a TrafficProtectionPolicy needed by the // mutation layer to inject Coraza WAF config. type TPPInfo struct { diff --git a/internal/extensionserver/mutate/tpp.go b/internal/extensionserver/mutate/tpp.go index 74fa8f47..6f5cb92d 100644 --- a/internal/extensionserver/mutate/tpp.go +++ b/internal/extensionserver/mutate/tpp.go @@ -3,6 +3,7 @@ package mutate import ( "encoding/json" "fmt" + "strconv" "strings" xdstypev3 "github.com/cncf/xds/go/xds/type/v3" @@ -156,7 +157,7 @@ func InjectCorazaListenerFilters(l *listenerv3.Listener, cfg *CorazaConfig) (int // 1. Extracts the EG filter_metadata["envoy-gateway"] gateway resource ref. // 2. Resolves the upstream namespace via idx.DStoUS. // 3. Stamps project_name into datum-gateway route metadata on every NSO-owned route. -// 4. Finds the governing TPP from idx.TPPs (route-level wins over gateway-level). +// 4. Finds the governing TPP from idx.TPPs (rule-level wins over route-level, which wins over gateway-level). // 5. Writes typed_per_filter_config and datum-gateway metadata on governed routes. // // applied, when non-nil, is populated with "namespace/name" → generation for @@ -204,11 +205,8 @@ func ApplyTPPRouteConfig( continue } - // Check for a route-level TPP (HTTPRoute targeting) — takes precedence. _, _, routeName, _ := extractEGResource(rt.GetMetadata()) - routeTPP := findRouteTPP(tpps, routeName) - - governing := routeTPP + governing := findRouteTPP(tpps, routeName, routeRuleName(rt, dsNS, routeName, idx)) if governing == nil { governing = gwTPP } @@ -295,20 +293,58 @@ func findGatewayTPP(tpps []extcache.TPPInfo, gatewayName string) *extcache.TPPIn return nil } -// findRouteTPP returns the first TPP in tpps whose TargetRefs includes an -// HTTPRoute target matching routeName. -func findRouteTPP(tpps []extcache.TPPInfo, routeName string) *extcache.TPPInfo { +func routeRuleName(rt *routev3.Route, dsNS, routeName string, idx *extcache.PolicyIndex) string { + if routeName == "" { + return "" + } + ruleIndex, ok := routeRuleIndex(rt, dsNS, routeName) + if !ok { + return "" + } + names := idx.HTTPProxyRules[extcache.HTTPProxyKey{Namespace: dsNS, Name: routeName}] + if ruleIndex >= len(names) { + return "" + } + return names[ruleIndex] +} + +func routeRuleIndex(rt *routev3.Route, dsNS, routeName string) (int, bool) { + if ns, name, i, ok := parseConnectorClusterName(routeCluster(rt)); ok && ns == dsNS && name == routeName { + return i, true + } + parts := strings.Split(rt.GetName(), "/") + if len(parts) < 5 || parts[0] != "httproute" || parts[1] != dsNS || parts[2] != routeName || parts[3] != "rule" { + return 0, false + } + i, err := strconv.Atoi(parts[4]) + if err != nil || i < 0 { + return 0, false + } + return i, true +} + +func findRouteTPP(tpps []extcache.TPPInfo, routeName, ruleName string) *extcache.TPPInfo { if routeName == "" { return nil } + var routeLevel *extcache.TPPInfo for i := range tpps { for _, ref := range tpps[i].TargetRefs { - if string(ref.Kind) == kindHTTPRoute && string(ref.Name) == routeName { + if string(ref.Kind) != kindHTTPRoute || string(ref.Name) != routeName { + continue + } + if ref.SectionName == nil || *ref.SectionName == "" { + if routeLevel == nil { + routeLevel = &tpps[i] + } + continue + } + if ruleName != "" && string(*ref.SectionName) == ruleName { return &tpps[i] } } } - return nil + return routeLevel } // --- EG metadata extraction --- diff --git a/internal/extensionserver/mutate/tpp_test.go b/internal/extensionserver/mutate/tpp_test.go index 89bd28fa..1bcc7e79 100644 --- a/internal/extensionserver/mutate/tpp_test.go +++ b/internal/extensionserver/mutate/tpp_test.go @@ -1,6 +1,7 @@ package mutate import ( + "fmt" "strings" "testing" @@ -605,3 +606,189 @@ func TestApplyTPPRouteConfig_RouteLevelTPPWins(t *testing.T) { // Route-level TPP mode is Enforce. assert.Equal(t, string(networkingv1alpha.TrafficProtectionPolicyEnforce), entry["mode"].GetStringValue()) } + +const sectionTestProxy = "alb" + +func sectionTPP(name string, mode networkingv1alpha.TrafficProtectionPolicyMode, section *string) extcache.TPPInfo { + ref := gatewayv1alpha2.LocalPolicyTargetReferenceWithSectionName{ + LocalPolicyTargetReference: gatewayv1.LocalPolicyTargetReference{ + Kind: "HTTPRoute", + Name: gatewayv1.ObjectName(sectionTestProxy), + }, + } + if section != nil { + ref.SectionName = (*gatewayv1.SectionName)(section) + } + return extcache.TPPInfo{ + Namespace: "test-project", + Name: name, + Generation: 1, + Mode: mode, + TargetRefs: []gatewayv1alpha2.LocalPolicyTargetReferenceWithSectionName{ref}, + Directives: []string{"SecRuleEngine On"}, + } +} + +func envoyRoute(t *testing.T, name, cluster string) *routev3.Route { + t.Helper() + rt := &routev3.Route{ + Name: name, + Metadata: &corev3.Metadata{FilterMetadata: map[string]*structpb.Struct{ + envoyGatewayMetadataKey: buildEGMetadataStruct("HTTPRoute", "ns-abc-123", sectionTestProxy), + }}, + } + if cluster != "" { + rt.Action = &routev3.Route_Route{Route: &routev3.RouteAction{ + ClusterSpecifier: &routev3.RouteAction_Cluster{Cluster: cluster}, + }} + } + return rt +} + +func governingTPPName(rt *routev3.Route) string { + md := rt.GetMetadata().GetFilterMetadata()[datumGatewayMetadataKey] + res := md.GetFields()["resources"].GetListValue().GetValues() + if len(res) == 0 { + return "" + } + return res[0].GetStructValue().GetFields()["name"].GetStringValue() +} + +func TestApplyTPPRouteConfig_SectionScoping(t *testing.T) { + const proxy = sectionTestProxy + str := func(s string) *string { return &s } + enforce := networkingv1alpha.TrafficProtectionPolicyEnforce + observe := networkingv1alpha.TrafficProtectionPolicyObserve + + routeName := func(rule int) string { + return fmt.Sprintf("httproute/ns-abc-123/%s/rule/%d/match/0/app_example_com", proxy, rule) + } + cluster := func(rule int) string { + return fmt.Sprintf("httproute/ns-abc-123/%s/rule/%d", proxy, rule) + } + + tests := []struct { + name string + rules []string + tpps []extcache.TPPInfo + want [3]string + }{ + { + name: "section-scoped applies only to its rule index", + rules: []string{"exempt", "protected", "other"}, + tpps: []extcache.TPPInfo{sectionTPP("sec", enforce, str("protected"))}, + want: [3]string{"", "sec", ""}, + }, + { + name: "section-scoped applies to a redirect-only rule without a cluster", + rules: []string{"exempt", "protected", "redirect"}, + tpps: []extcache.TPPInfo{sectionTPP("sec", enforce, str("redirect"))}, + want: [3]string{"", "", "sec"}, + }, + { + name: "route-level applies to all rules", + rules: []string{"exempt", "protected", "other"}, + tpps: []extcache.TPPInfo{sectionTPP("route", enforce, nil)}, + want: [3]string{"route", "route", "route"}, + }, + { + name: "rule-level beats route-level regardless of order", + rules: []string{"exempt", "protected", "other"}, + tpps: []extcache.TPPInfo{ + sectionTPP("route", observe, nil), + sectionTPP("sec", enforce, str("protected")), + }, + want: [3]string{"route", "sec", "route"}, + }, + { + name: "rule-level beats gateway-level", + rules: []string{"exempt", "protected", "other"}, + tpps: []extcache.TPPInfo{ + tppTargetingGateway("gw", "smoke-gw"), + sectionTPP("sec", enforce, str("protected")), + }, + want: [3]string{"gw", "sec", "gw"}, + }, + { + name: "unresolvable section applies to nothing", + rules: []string{"exempt", "protected", "other"}, + tpps: []extcache.TPPInfo{sectionTPP("sec", enforce, str("missing"))}, + want: [3]string{"", "", ""}, + }, + { + name: "unresolvable section never widens to other rules but gateway-level still governs", + rules: []string{"exempt", "protected", "other"}, + tpps: []extcache.TPPInfo{ + tppTargetingGateway("gw", "smoke-gw"), + sectionTPP("sec", enforce, str("missing")), + }, + want: [3]string{"gw", "gw", "gw"}, + }, + { + name: "unknown HTTPProxy applies section to nothing", + rules: nil, + tpps: []extcache.TPPInfo{sectionTPP("sec", enforce, str("protected"))}, + want: [3]string{"", "", ""}, + }, + { + name: "unnamed rules are not matched by a named section", + rules: []string{"", "protected", ""}, + tpps: []extcache.TPPInfo{sectionTPP("sec", enforce, str("protected"))}, + want: [3]string{"", "sec", ""}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + idx := policyIndex(tt.tpps...) + if tt.rules != nil { + idx.HTTPProxyRules = map[extcache.HTTPProxyKey][]string{ + {Namespace: "ns-abc-123", Name: proxy}: tt.rules, + } + } + + routes := []*routev3.Route{ + envoyRoute(t, routeName(0), cluster(0)), + envoyRoute(t, routeName(1), cluster(1)), + envoyRoute(t, routeName(2), ""), + } + rc := &routev3.RouteConfiguration{VirtualHosts: []*routev3.VirtualHost{buildVHWithGatewayMeta(routes...)}} + + _, err := ApplyTPPRouteConfig(rc, idx, testCorazaConfig(), nil) + require.NoError(t, err) + for i, rt := range routes { + assert.Equal(t, tt.want[i], governingTPPName(rt), "rule %d", i) + } + }) + } +} + +func TestRouteRuleIndex(t *testing.T) { + tests := []struct { + name string + routeName string + cluster string + wantIdx int + wantOK bool + }{ + {name: "from cluster", routeName: "unrelated", cluster: "httproute/ns-abc-123/alb/rule/3", wantIdx: 3, wantOK: true}, + {name: "from route name", routeName: "httproute/ns-abc-123/alb/rule/2/match/1/host", wantIdx: 2, wantOK: true}, + {name: "route name without host", routeName: "httproute/ns-abc-123/alb/rule/12/match/0", wantIdx: 12, wantOK: true}, + {name: "cluster of another route", routeName: "x", cluster: "httproute/ns-abc-123/other/rule/1"}, + {name: "route name of another namespace", routeName: "httproute/other/alb/rule/1/match/0"}, + {name: "non-numeric index", routeName: "httproute/ns-abc-123/alb/rule/x/match/0"}, + {name: "negative index", routeName: "httproute/ns-abc-123/alb/rule/-1/match/0"}, + {name: "not an httproute", routeName: "tcproute/ns-abc-123/alb/rule/1"}, + {name: "empty", routeName: ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rt := envoyRoute(t, tt.routeName, tt.cluster) + got, ok := routeRuleIndex(rt, "ns-abc-123", sectionTestProxy) + assert.Equal(t, tt.wantOK, ok) + if tt.wantOK { + assert.Equal(t, tt.wantIdx, got) + } + }) + } +} diff --git a/internal/webhook/v1alpha/trafficprotectionpolicy_webhook.go b/internal/webhook/v1alpha/trafficprotectionpolicy_webhook.go index 86b3f127..26d59a64 100644 --- a/internal/webhook/v1alpha/trafficprotectionpolicy_webhook.go +++ b/internal/webhook/v1alpha/trafficprotectionpolicy_webhook.go @@ -7,9 +7,11 @@ import ( "encoding/json" "fmt" + "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -25,6 +27,7 @@ import ( func SetupTrafficProtectionPolicyWebhookWithManager(mgr mcmanager.Manager) error { return ctrl.NewWebhookManagedBy(mgr.GetLocalManager(), &networkingv1alpha.TrafficProtectionPolicy{}). WithDefaulter(&TrafficProtectionPolicyDefaulter{mgr: mgr}). + WithValidator(&TrafficProtectionPolicyValidator{mgr: mgr}). Complete() } @@ -43,14 +46,21 @@ func (d *TrafficProtectionPolicyDefaulter) Default(ctx context.Context, policy * } func (d *TrafficProtectionPolicyDefaulter) clusterClient(ctx context.Context) client.Client { - if d == nil || d.mgr == nil { + if d == nil { + return nil + } + return webhookClusterClient(ctx, d.mgr) +} + +func webhookClusterClient(ctx context.Context, mgr mcmanager.Manager) client.Client { + if mgr == nil { return nil } clusterName, ok := mccontext.ClusterFrom(ctx) if !ok { - return d.mgr.GetLocalManager().GetClient() + return mgr.GetLocalManager().GetClient() } - cluster, err := d.mgr.GetCluster(ctx, clusterName) + cluster, err := mgr.GetCluster(ctx, clusterName) if err != nil { return nil } @@ -127,3 +137,71 @@ func httpProxyByName(ctx context.Context, cl client.Client, key types.Namespaced } return &proxy } + +// +kubebuilder:webhook:path=/validate-networking-datumapis-com-v1alpha-trafficprotectionpolicy,mutating=false,failurePolicy=fail,sideEffects=None,groups=networking.datumapis.com,resources=trafficprotectionpolicies,verbs=create;update,versions=v1alpha,name=vtrafficprotectionpolicy-v1alpha.kb.io,admissionReviewVersions=v1 + +type TrafficProtectionPolicyValidator struct { + mgr mcmanager.Manager +} + +var _ admission.Validator[*networkingv1alpha.TrafficProtectionPolicy] = &TrafficProtectionPolicyValidator{} + +func (v *TrafficProtectionPolicyValidator) ValidateCreate(ctx context.Context, policy *networkingv1alpha.TrafficProtectionPolicy) (admission.Warnings, error) { + return v.validateSectionNames(ctx, policy) +} + +func (v *TrafficProtectionPolicyValidator) ValidateUpdate(ctx context.Context, oldPolicy, newPolicy *networkingv1alpha.TrafficProtectionPolicy) (admission.Warnings, error) { + if equality.Semantic.DeepEqual(oldPolicy.Spec.TargetRefs, newPolicy.Spec.TargetRefs) { + return nil, nil + } + return v.validateSectionNames(ctx, newPolicy) +} + +func (v *TrafficProtectionPolicyValidator) ValidateDelete(context.Context, *networkingv1alpha.TrafficProtectionPolicy) (admission.Warnings, error) { + return nil, nil +} + +func (v *TrafficProtectionPolicyValidator) validateSectionNames(ctx context.Context, policy *networkingv1alpha.TrafficProtectionPolicy) (admission.Warnings, error) { + cl := webhookClusterClient(ctx, v.mgr) + if cl == nil { + return nil, nil + } + return v.validateSectionNamesWithClient(ctx, cl, policy) +} + +func (v *TrafficProtectionPolicyValidator) validateSectionNamesWithClient(ctx context.Context, cl client.Client, policy *networkingv1alpha.TrafficProtectionPolicy) (admission.Warnings, error) { + var warnings admission.Warnings + var errs field.ErrorList + for i, ref := range policy.Spec.TargetRefs { + if ref.Kind != "HTTPRoute" || ref.SectionName == nil { + continue + } + path := field.NewPath("spec", "targetRefs").Index(i).Child("sectionName") + + proxy := httpProxyByName(ctx, cl, types.NamespacedName{Namespace: policy.Namespace, Name: string(ref.Name)}) + if proxy == nil { + warnings = append(warnings, fmt.Sprintf("%s: HTTPProxy %q was not found, so rule %q could not be verified", path, ref.Name, *ref.SectionName)) + continue + } + + hasRule := false + ruleNames := make([]string, 0, len(proxy.Spec.Rules)) + for _, rule := range proxy.Spec.Rules { + if rule.Name == nil { + continue + } + ruleNames = append(ruleNames, string(*rule.Name)) + if *rule.Name == *ref.SectionName { + hasRule = true + } + } + if !hasRule { + errs = append(errs, field.NotFound(path, fmt.Sprintf("%s (HTTPProxy %q has rules %v)", *ref.SectionName, ref.Name, ruleNames))) + } + } + + if len(errs) > 0 { + return warnings, apierrors.NewInvalid(policy.GroupVersionKind().GroupKind(), policy.Name, errs) + } + return warnings, nil +} diff --git a/internal/webhook/v1alpha/trafficprotectionpolicy_webhook_test.go b/internal/webhook/v1alpha/trafficprotectionpolicy_webhook_test.go index 44f55b97..14c100e5 100644 --- a/internal/webhook/v1alpha/trafficprotectionpolicy_webhook_test.go +++ b/internal/webhook/v1alpha/trafficprotectionpolicy_webhook_test.go @@ -10,6 +10,8 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -79,3 +81,81 @@ func ptrTrue() *bool { t := true return &t } + +func TestTrafficProtectionPolicyValidator_SectionNames(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + require.NoError(t, networkingv1alpha.AddToScheme(scheme)) + require.NoError(t, gatewayv1.Install(scheme)) + + ruleName := func(s string) *gatewayv1.SectionName { return ptr.To(gatewayv1.SectionName(s)) } + proxy := &networkingv1alpha.HTTPProxy{ + ObjectMeta: metav1.ObjectMeta{Name: "alb", Namespace: "proj"}, + Spec: networkingv1alpha.HTTPProxySpec{ + Rules: []networkingv1alpha.HTTPProxyRule{ + {Name: ruleName("exempt")}, + {Name: ruleName("protected")}, + {}, + }, + }, + } + + policyWithRef := func(kind, name string, section *gatewayv1.SectionName) *networkingv1alpha.TrafficProtectionPolicy { + p := tppPolicy() + p.Spec.TargetRefs = []gatewayv1alpha2.LocalPolicyTargetReferenceWithSectionName{{ + LocalPolicyTargetReference: gatewayv1.LocalPolicyTargetReference{ + Group: gatewayv1.GroupName, + Kind: gatewayv1.Kind(kind), + Name: gatewayv1.ObjectName(name), + }, + SectionName: section, + }} + return p + } + + tests := []struct { + name string + objs []client.Object + policy *networkingv1alpha.TrafficProtectionPolicy + wantErr string + wantWarnings int + }{ + {name: "existing rule accepted", objs: []client.Object{proxy}, policy: policyWithRef("HTTPRoute", "alb", ruleName("protected"))}, + {name: "missing rule rejected", objs: []client.Object{proxy}, policy: policyWithRef("HTTPRoute", "alb", ruleName("nope")), wantErr: "spec.targetRefs[0].sectionName"}, + {name: "HTTPProxy absent only warns", policy: policyWithRef("HTTPRoute", "alb", ruleName("protected")), wantWarnings: 1}, + {name: "route-level ref not validated", objs: []client.Object{proxy}, policy: policyWithRef("HTTPRoute", "alb", nil)}, + {name: "gateway listener sectionName not validated", objs: []client.Object{proxy}, policy: policyWithRef("Gateway", "alb", ruleName("https"))}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.objs...).Build() + v := &TrafficProtectionPolicyValidator{} + + warnings, err := v.validateSectionNamesWithClient(context.Background(), cl, tt.policy) + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + } else { + require.NoError(t, err) + } + assert.Len(t, warnings, tt.wantWarnings) + }) + } +} + +func TestTrafficProtectionPolicyValidator_UpdateSkipsUnchangedTargetRefs(t *testing.T) { + t.Parallel() + + old := tppPolicy() + old.Spec.TargetRefs[0].Kind = "HTTPRoute" + old.Spec.TargetRefs[0].SectionName = ptr.To(gatewayv1.SectionName("stale")) + updated := old.DeepCopy() + updated.Spec.Mode = networkingv1alpha.TrafficProtectionPolicyEnforce + + warnings, err := (&TrafficProtectionPolicyValidator{}).ValidateUpdate(context.Background(), old, updated) + require.NoError(t, err) + assert.Empty(t, warnings) +} diff --git a/test/e2e-edge/waf-section-scoping/chainsaw-test.yaml b/test/e2e-edge/waf-section-scoping/chainsaw-test.yaml new file mode 100644 index 00000000..99d0a654 --- /dev/null +++ b/test/e2e-edge/waf-section-scoping/chainsaw-test.yaml @@ -0,0 +1,220 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +# +# Section-scoped firewall policy. +# +# An HTTPProxy with two named rules ("exempt" on /anything/exempt and +# "protected" on /) carries a TrafficProtectionPolicy whose targetRef names the +# "protected" rule through sectionName. The policy must be Accepted, block a +# SQLi probe on the protected rule with 403, and leave the exempt rule +# untouched: the same probe on /anything/exempt must return 200. +# +# Assertions are on real HTTP responses from the edge, never on status alone, +# because a policy can be accepted yet applied to the wrong routes. The +# HTTPProxy is used (rather than a hand-built HTTPRoute) because rule names +# live on the HTTPProxy; the project control plane's HTTPRoute CRD has no +# rules[].name. +# +# Runs alone (concurrent: false) because it shares cluster-wide downstream +# config with the other data-plane tests. Cluster names are inline; the +# downstream gateway class is provisioned by the test environment. +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: waf-section-scoping +spec: + concurrent: false + cluster: nso-upstream + bindings: + - name: downstreamGatewayClass + value: datum-downstream-gateway-e2e + steps: + - name: Create the echo backend + try: + - apply: + cluster: nso-downstream + file: ../_fixtures/echo-backend.yaml + - apply: + cluster: nso-downstream + resource: + apiVersion: v1 + kind: Namespace + metadata: + name: datum-downstream-gateway-hostnames + - assert: + cluster: nso-downstream + resource: + apiVersion: v1 + kind: Pod + metadata: + name: echo-backend + namespace: default + status: + phase: Running + + - name: Provision an HTTPProxy with two named rules and a section-scoped policy + try: + - create: + resource: + apiVersion: networking.datumapis.com/v1alpha + kind: HTTPProxy + metadata: + name: sectioned-proxy + spec: + rules: + - name: exempt + matches: + - path: + type: PathPrefix + value: /anything/exempt + backends: + - endpoint: http://echo-backend.default.svc.cluster.local:8080 + - name: protected + matches: + - path: + type: PathPrefix + value: / + backends: + - endpoint: http://echo-backend.default.svc.cluster.local:8080 + - create: + resource: + apiVersion: networking.datumapis.com/v1alpha + kind: TrafficProtectionPolicy + metadata: + name: protected-only + spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: sectioned-proxy + sectionName: protected + mode: Enforce + ruleSets: + - type: OWASPCoreRuleSet + owaspCoreRuleSet: {} + - script: + timeout: 180s + content: | + set -eu + for i in $(seq 1 36); do + H=$(kubectl -n $NAMESPACE get httpproxy sectioned-proxy \ + -o jsonpath='{.status.canonicalHostname}' 2>/dev/null || true) + if [ -n "${H}" ]; then echo "canonical hostname: ${H}"; exit 0; fi + echo "waiting for HTTPProxy canonicalHostname... ${i}" >&2 + sleep 5 + done + echo "ERROR: HTTPProxy never published a canonicalHostname" >&2 + exit 1 + - script: + timeout: 120s + content: | + set -eu + for i in $(seq 1 24); do + S=$(kubectl -n $NAMESPACE get trafficprotectionpolicy protected-only \ + -o jsonpath='{.status.ancestors[0].conditions[?(@.type=="Accepted")].status}' 2>/dev/null || true) + if [ "${S}" = "True" ]; then echo "policy Accepted"; exit 0; fi + echo "waiting for policy Accepted (got '${S}')... ${i}" >&2 + sleep 5 + done + echo "ERROR: section-scoped policy was not Accepted" >&2 + kubectl -n $NAMESPACE get trafficprotectionpolicy protected-only -o yaml >&2 + exit 1 + catch: + - script: + content: | + kubectl -n network-services-operator-system logs -l app.kubernetes.io/name=network-services-operator --tail=200 + kubectl -n $NAMESPACE get httpproxy,httproute,trafficprotectionpolicy -o yaml + + - name: Stand up the long-lived conntest client + try: + - create: + cluster: nso-downstream + resource: + apiVersion: v1 + kind: Pod + metadata: + name: conntest + namespace: default + spec: + containers: + - name: curl + image: curlimages/curl:8.11.1 + command: ["sleep", "infinity"] + resources: + requests: + cpu: 50m + memory: 64Mi + limits: + cpu: 100m + memory: 128Mi + terminationGracePeriodSeconds: 0 + - assert: + cluster: nso-downstream + resource: + apiVersion: v1 + kind: Pod + metadata: + name: conntest + namespace: default + status: + phase: Running + + - name: The policy blocks the protected rule and leaves the exempt rule alone + description: | + Plain HTTP on port 80 is probed because the HTTPS listener has no issued + certificate in this environment; the firewall applies per route on both + listeners. Each probe retries until the edge has picked up the config. + The exempt path must return 200 even for the SQLi payload; the protected + path must return 403 for it and 200 for a benign request. + try: + - script: + timeout: 120s + skipCommandOutput: true + skipLogOutput: true + content: | + kubectl -n $NAMESPACE get httpproxy sectioned-proxy -o jsonpath='{.status.canonicalHostname}' + outputs: + - name: proxyHostname + value: ($stdout) + - script: + timeout: 420s + cluster: nso-downstream + env: + - name: HOST + value: ($proxyHostname) + - name: OWNING_GC + value: ($downstreamGatewayClass) + content: | + set -eu + IP=$(kubectl get svc -n datum-downstream-gateway \ + -l gateway.envoyproxy.io/owning-gatewayclass=$OWNING_GC \ + -o jsonpath='{.items[0].spec.clusterIP}') + echo "host: ${HOST} edge IP: ${IP}" + SQLI='id=1%27%20OR%20%271%27%3D%271' + + probe() { + want="$1"; path="$2" + for i in $(seq 1 30); do + code=$(kubectl -n default exec conntest -- \ + curl -ksS -o /dev/null -w '%{http_code}' --max-time 10 \ + --resolve "${HOST}:80:${IP}" \ + "http://${HOST}${path}" 2>/dev/null || true) + echo "attempt ${i}: ${path} -> ${code} (want ${want})" + [ "${code}" = "${want}" ] && return 0 + sleep 5 + done + echo "ERROR: ${path} expected ${want}, last status ${code}" + return 1 + } + + probe 403 "/get?${SQLI}" + probe 200 "/get" + probe 200 "/anything/exempt?${SQLI}" + echo "OK: protected rule blocked, exempt rule untouched" + catch: + - script: + cluster: nso-downstream + env: + - name: OWNING_GC + value: ($downstreamGatewayClass) + content: | + kubectl -n datum-downstream-gateway logs -l gateway.envoyproxy.io/owning-gatewayclass=$OWNING_GC -c envoy --tail=100