From 622ad24ec89e8834613465776142787417e404e9 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Thu, 23 Apr 2026 14:16:38 -0700 Subject: [PATCH 1/2] feat: add TEG ExtendedSecurityPolicy WAF backend for TrafficProtectionPolicy Adds a new teg-esp WAF backend mode to the TrafficProtectionPolicy controller that emits ExtendedSecurityPolicy resources (teg.tetrate.io/v1alpha1) for the Tetrate Enterprise Gateway WAF instead of the existing Coraza Go filter + EnvoyPatchPolicy approach. Controlled by gateway.coraza.backend in operator config: - coraza-epp (default): existing EPP path, no behavioral change - teg-esp: emits one ExtendedSecurityPolicy per Gateway targeting the downstream Gateway by name, with WAF directives as a flat SecLang string in spec.waf.directives Also restricts TPP targetRefs to Gateway/GatewayClass only (removes HTTPRoute), matching the per-gateway granularity of ESP, and regenerates the CRD manifest accordingly. --- api/v1alpha/trafficprotectionpolicy_types.go | 2 +- ...tumapis.com_trafficprotectionpolicies.yaml | 6 +- internal/config/config.go | 19 + internal/config/zz_generated.defaults.go | 3 + .../trafficprotectionpolicy_controller.go | 277 ++++++++++---- ...trafficprotectionpolicy_controller_test.go | 349 +++++++++++++++++- 6 files changed, 579 insertions(+), 77 deletions(-) diff --git a/api/v1alpha/trafficprotectionpolicy_types.go b/api/v1alpha/trafficprotectionpolicy_types.go index 59699e67..9e3d4679 100644 --- a/api/v1alpha/trafficprotectionpolicy_types.go +++ b/api/v1alpha/trafficprotectionpolicy_types.go @@ -28,7 +28,7 @@ const ( // TrafficProtectionPolicySpec defines the desired state of TrafficProtectionPolicy. // // +kubebuilder:validation:XValidation:rule="has(self.targetRefs) ? self.targetRefs.all(ref, ref.group == 'gateway.networking.k8s.io') : true ", message="this policy can only have a targetRefs[*].group of gateway.networking.k8s.io" -// +kubebuilder:validation:XValidation:rule="has(self.targetRefs) ? self.targetRefs.all(ref, ref.kind in ['Gateway', 'HTTPRoute']) : true ", message="this policy can only have a targetRefs[*].kind of Gateway/HTTPRoute" +// +kubebuilder:validation:XValidation:rule="has(self.targetRefs) ? self.targetRefs.all(ref, ref.kind == 'Gateway') : true", message="this policy can only have a targetRefs[*].kind of Gateway" type TrafficProtectionPolicySpec struct { // TargetRefs are the names of the Gateway resources this policy diff --git a/config/crd/bases/networking.datumapis.com_trafficprotectionpolicies.yaml b/config/crd/bases/networking.datumapis.com_trafficprotectionpolicies.yaml index 3458ca7b..b8ba2d82 100644 --- a/config/crd/bases/networking.datumapis.com_trafficprotectionpolicies.yaml +++ b/config/crd/bases/networking.datumapis.com_trafficprotectionpolicies.yaml @@ -237,9 +237,9 @@ spec: - message: this policy can only have a targetRefs[*].group of gateway.networking.k8s.io rule: 'has(self.targetRefs) ? self.targetRefs.all(ref, ref.group == ''gateway.networking.k8s.io'') : true ' - - message: this policy can only have a targetRefs[*].kind of Gateway/HTTPRoute - rule: 'has(self.targetRefs) ? self.targetRefs.all(ref, ref.kind in [''Gateway'', - ''HTTPRoute'']) : true ' + - message: this policy can only have a targetRefs[*].kind of Gateway + rule: 'has(self.targetRefs) ? self.targetRefs.all(ref, ref.kind == ''Gateway'') + : true' status: description: TrafficProtectionPolicyStatus defines the observed state of TrafficProtectionPolicy. diff --git a/internal/config/config.go b/internal/config/config.go index 0b9e3c70..1d01c74f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -645,6 +645,16 @@ func (c *GatewayConfig) ConnectorTunnelListenerName() string { return c.ConnectorInternalListenerName } +// WAFBackend identifies which downstream WAF resource the operator emits. +type WAFBackend string + +const ( + // WAFBackendCorazaEPP uses EnvoyPatchPolicy with the Coraza Go filter. + WAFBackendCorazaEPP WAFBackend = "coraza-epp" + // WAFBackendTEGESP uses ExtendedSecurityPolicy from Tetrate Enterprise Gateway. + WAFBackendTEGESP WAFBackend = "teg-esp" +) + // +k8s:deepcopy-gen=true type CorazaConfig struct { @@ -685,6 +695,15 @@ type CorazaConfig struct { // stored in Envoy routes to inject into trace span attributes. MUST return // a map of string keys to values. TraceRouteMetadataExtractor string `json:"traceRouteMetadataExtractor,omitempty"` + + // WAFBackend selects which downstream resource the TrafficProtectionPolicy + // controller emits. "coraza-epp" (default) writes EnvoyPatchPolicy resources + // using the Coraza Go filter. "teg-esp" writes ExtendedSecurityPolicy resources + // for the Tetrate Enterprise Gateway WAF. + // + // +kubebuilder:validation:Enum=coraza-epp;teg-esp + // +default="coraza-epp" + Backend WAFBackend `json:"backend,omitempty"` } // +k8s:deepcopy-gen=true diff --git a/internal/config/zz_generated.defaults.go b/internal/config/zz_generated.defaults.go index 65817ce3..4001dc31 100644 --- a/internal/config/zz_generated.defaults.go +++ b/internal/config/zz_generated.defaults.go @@ -57,6 +57,9 @@ func SetObjectDefaults_NetworkServicesOperator(in *NetworkServicesOperator) { panic(err) } } + if in.Gateway.Coraza.Backend == "" { + in.Gateway.Coraza.Backend = WAFBackendCorazaEPP + } if in.Gateway.ValidPortNumbers == nil { if err := json.Unmarshal([]byte(`[80,443]`), &in.Gateway.ValidPortNumbers); err != nil { panic(err) diff --git a/internal/controller/trafficprotectionpolicy_controller.go b/internal/controller/trafficprotectionpolicy_controller.go index 9bbe8c8b..681f82a2 100644 --- a/internal/controller/trafficprotectionpolicy_controller.go +++ b/internal/controller/trafficprotectionpolicy_controller.go @@ -17,6 +17,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" @@ -72,6 +73,13 @@ const ( tppManagedLabel = "networking.datumapis.com/managed-by-tpp-controller" ) +// tegESPGVK is the GroupVersionKind for TEG's ExtendedSecurityPolicy CRD. +var tegESPGVK = schema.GroupVersionKind{ + Group: "teg.tetrate.io", + Version: "v1alpha1", + Kind: "ExtendedSecurityPolicy", +} + // certificateReadinessResult contains the result of checking certificate readiness // for HTTPS listeners. type certificateReadinessResult struct { @@ -86,13 +94,20 @@ type certificateReadinessResult struct { // +kubebuilder:rbac:groups=networking.datumapis.com,resources=trafficprotectionpolicies/finalizers,verbs=update // +kubebuilder:rbac:groups=cert-manager.io,resources=certificates,verbs=get;list;watch +func (r *TrafficProtectionPolicyReconciler) isTEGMode() bool { + return r.Config.Gateway.Coraza.Backend == config.WAFBackendTEGESP +} + func (r *TrafficProtectionPolicyReconciler) Reconcile(ctx context.Context, req NamespaceReconcileRequest) (ctrl.Result, error) { logger := log.FromContext(ctx, "cluster", req.ClusterName) ctx = log.IntoContext(ctx, logger) // Ensure that the HTTP listener has the Coraza WAF filter configured. - if err := r.ensureHTTPCorazaListenerFilter(ctx); err != nil { - return ctrl.Result{}, err + // Skip this step in TEG mode — no Coraza EPP is needed. + if !r.isTEGMode() { + if err := r.ensureHTTPCorazaListenerFilter(ctx); err != nil { + return ctrl.Result{}, err + } } cl, err := r.mgr.GetCluster(ctx, req.ClusterName) @@ -136,95 +151,152 @@ func (r *TrafficProtectionPolicyReconciler) Reconcile(ctx context.Context, req N attachments := r.collectTrafficProtectionPolicyAttachments(ctx, trafficProtectionPolicies, upstreamGateways.Items, upstreamHTTPRoutes.Items) - // Check if all HTTPS listener certificates are ready before creating EnvoyPatchPolicies. - // This prevents JSONPath selector failures when Envoy Gateway hasn't materialized filter_chains. - certReadiness, err := r.checkHTTPSListenerCertificatesReady(ctx, downstreamNamespaceName, attachments) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to check certificate readiness: %w", err) - } + if r.isTEGMode() { + // TEG mode: emit ExtendedSecurityPolicy resources instead of EnvoyPatchPolicies. + desiredESPs := r.getDesiredExtendedSecurityPolicies(downstreamNamespaceName, attachments) - if !certReadiness.AllReady { - logger.Info("waiting for TLS certificates to become ready", "pendingListeners", certReadiness.PendingListeners) - r.setWaitingForCertificatesConditions(trafficProtectionPolicies, certReadiness.PendingListeners) + desiredESPNames := make(map[string]struct{}, len(desiredESPs)) + for _, esp := range desiredESPs { + desiredESPNames[esp.GetName()] = struct{}{} - if err := r.updateTPPAncestorsStatus(ctx, cl.GetClient(), trafficProtectionPolicies, originalTrafficProtectionPolicies); err != nil { - return ctrl.Result{}, err - } + existing := newUnstructuredForGVK(tegESPGVK) + existing.SetNamespace(esp.GetNamespace()) + existing.SetName(esp.GetName()) - // Certificate watch will trigger reconciliation when certificates become ready - return ctrl.Result{}, nil - } + result, err := controllerutil.CreateOrUpdate(ctx, downstreamStrategy.GetClient(), existing, func() error { + if existing.GetLabels() == nil { + existing.SetLabels(make(map[string]string)) + } + labels := existing.GetLabels() + labels[tppManagedLabel] = "true" + existing.SetLabels(labels) + existing.Object["spec"] = esp.Object["spec"] + return nil + }) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to create or update extendedsecuritypolicy %s/%s: %w", esp.GetNamespace(), esp.GetName(), err) + } + logger.Info("applied extendedsecuritypolicy to downstream cluster", "namespace", esp.GetNamespace(), "name", esp.GetName(), "result", result) + } - listenerReadiness := r.checkHTTPSListenersProgrammed(attachments) - if !listenerReadiness.AllReady { - logger.Info("waiting for HTTPS listeners to become programmed", "pendingListeners", listenerReadiness.PendingListeners) - r.setWaitingForListenersProgrammedConditions(trafficProtectionPolicies, listenerReadiness.PendingListeners) + // Clean up stale ESPs by label. + existingESPList := &unstructured.UnstructuredList{} + existingESPList.SetGroupVersionKind(schema.GroupVersionKind{ + Group: tegESPGVK.Group, + Version: tegESPGVK.Version, + Kind: tegESPGVK.Kind + "List", + }) + if err := downstreamStrategy.GetClient().List( + ctx, + existingESPList, + client.InNamespace(downstreamNamespaceName), + client.MatchingLabels{tppManagedLabel: "true"}, + ); err != nil && !apierrors.IsNotFound(err) { + return ctrl.Result{}, fmt.Errorf("failed to list extendedsecuritypolicies: %w", err) + } + for i := range existingESPList.Items { + existing := &existingESPList.Items[i] + if _, ok := desiredESPNames[existing.GetName()]; ok { + continue + } + if err := downstreamStrategy.GetClient().Delete(ctx, existing); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to delete stale extendedsecuritypolicy %s/%s: %w", existing.GetNamespace(), existing.GetName(), err) + } + logger.Info("deleted stale extendedsecuritypolicy from downstream cluster", "namespace", existing.GetNamespace(), "name", existing.GetName()) + } + } else { + // Coraza EPP mode (default): check cert and listener readiness before writing EPPs. - if err := r.updateTPPAncestorsStatus(ctx, cl.GetClient(), trafficProtectionPolicies, originalTrafficProtectionPolicies); err != nil { - return ctrl.Result{}, err + // Check if all HTTPS listener certificates are ready before creating EnvoyPatchPolicies. + // This prevents JSONPath selector failures when Envoy Gateway hasn't materialized filter_chains. + certReadiness, err := r.checkHTTPSListenerCertificatesReady(ctx, downstreamNamespaceName, attachments) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to check certificate readiness: %w", err) } - // Gateway/HTTPRoute watches will trigger reconciliation when listener status changes. - return ctrl.Result{}, nil - } + if !certReadiness.AllReady { + logger.Info("waiting for TLS certificates to become ready", "pendingListeners", certReadiness.PendingListeners) + r.setWaitingForCertificatesConditions(trafficProtectionPolicies, certReadiness.PendingListeners) - desiredPolicies, err := r.getDesiredEnvoyPatchPolicies(downstreamNamespaceName, attachments) - if err != nil { - return ctrl.Result{}, err - } + if err := r.updateTPPAncestorsStatus(ctx, cl.GetClient(), trafficProtectionPolicies, originalTrafficProtectionPolicies); err != nil { + return ctrl.Result{}, err + } - desiredPolicyNames := make(map[string]struct{}, len(desiredPolicies)) - for _, desiredPolicy := range desiredPolicies { - desiredPolicyNames[desiredPolicy.Name] = struct{}{} + // Certificate watch will trigger reconciliation when certificates become ready + return ctrl.Result{}, nil + } - policy := envoygatewayv1alpha1.EnvoyPatchPolicy{ObjectMeta: metav1.ObjectMeta{ - Namespace: desiredPolicy.Namespace, - Name: desiredPolicy.Name, - }} + listenerReadiness := r.checkHTTPSListenersProgrammed(attachments) + if !listenerReadiness.AllReady { + logger.Info("waiting for HTTPS listeners to become programmed", "pendingListeners", listenerReadiness.PendingListeners) + r.setWaitingForListenersProgrammedConditions(trafficProtectionPolicies, listenerReadiness.PendingListeners) - result, err := controllerutil.CreateOrUpdate(ctx, downstreamStrategy.GetClient(), &policy, func() error { - if policy.Labels == nil { - policy.Labels = make(map[string]string) + if err := r.updateTPPAncestorsStatus(ctx, cl.GetClient(), trafficProtectionPolicies, originalTrafficProtectionPolicies); err != nil { + return ctrl.Result{}, err } - policy.Labels[tppManagedLabel] = "true" - policy.Spec = desiredPolicy.Spec - return nil - }) + + // Gateway/HTTPRoute watches will trigger reconciliation when listener status changes. + return ctrl.Result{}, nil + } + + desiredPolicies, err := r.getDesiredEnvoyPatchPolicies(downstreamNamespaceName, attachments) if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to create or update envoypatchpolicy %s/%s: %w", policy.Namespace, policy.Name, err) + return ctrl.Result{}, err } - logger.Info("applied envoypatchpolicy to downstream cluster", "namespace", policy.Namespace, "name", policy.Name, "result", result) - } - // Clean up stale EPPs. All EPPs written by this controller are named - // "tpp-"; other controllers use different prefixes (e.g. - // "connector-" from the HTTPProxy controller). Filtering by prefix - // avoids a label dependency and correctly handles deleted gateways whose EPP - // would be missed if we only iterated upstreamGateways. - // TODO: once all existing EPPs carry tppManagedLabel (stamped above on every - // CreateOrUpdate), switch this List to use a label selector and drop the - // prefix check. - var existingPolicies envoygatewayv1alpha1.EnvoyPatchPolicyList - if err := downstreamStrategy.GetClient().List( - ctx, - &existingPolicies, - client.InNamespace(downstreamNamespaceName), - ); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to list envoypatchpolicies: %w", err) - } + desiredPolicyNames := make(map[string]struct{}, len(desiredPolicies)) + for _, desiredPolicy := range desiredPolicies { + desiredPolicyNames[desiredPolicy.Name] = struct{}{} - for i := range existingPolicies.Items { - existing := &existingPolicies.Items[i] - if !strings.HasPrefix(existing.Name, tppEnvoyPatchPolicyPrefix) { - continue + policy := envoygatewayv1alpha1.EnvoyPatchPolicy{ObjectMeta: metav1.ObjectMeta{ + Namespace: desiredPolicy.Namespace, + Name: desiredPolicy.Name, + }} + + result, err := controllerutil.CreateOrUpdate(ctx, downstreamStrategy.GetClient(), &policy, func() error { + if policy.Labels == nil { + policy.Labels = make(map[string]string) + } + policy.Labels[tppManagedLabel] = "true" + policy.Spec = desiredPolicy.Spec + return nil + }) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to create or update envoypatchpolicy %s/%s: %w", policy.Namespace, policy.Name, err) + } + logger.Info("applied envoypatchpolicy to downstream cluster", "namespace", policy.Namespace, "name", policy.Name, "result", result) } - if _, ok := desiredPolicyNames[existing.Name]; ok { - continue + + // Clean up stale EPPs. All EPPs written by this controller are named + // "tpp-"; other controllers use different prefixes (e.g. + // "connector-" from the HTTPProxy controller). Filtering by prefix + // avoids a label dependency and correctly handles deleted gateways whose EPP + // would be missed if we only iterated upstreamGateways. + // TODO: once all existing EPPs carry tppManagedLabel (stamped above on every + // CreateOrUpdate), switch this List to use a label selector and drop the + // prefix check. + var existingPolicies envoygatewayv1alpha1.EnvoyPatchPolicyList + if err := downstreamStrategy.GetClient().List( + ctx, + &existingPolicies, + client.InNamespace(downstreamNamespaceName), + ); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to list envoypatchpolicies: %w", err) } - if err := downstreamStrategy.GetClient().Delete(ctx, existing); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to delete stale envoypatchpolicy %s/%s: %w", existing.Namespace, existing.Name, err) + + for i := range existingPolicies.Items { + existing := &existingPolicies.Items[i] + if !strings.HasPrefix(existing.Name, tppEnvoyPatchPolicyPrefix) { + continue + } + if _, ok := desiredPolicyNames[existing.Name]; ok { + continue + } + if err := downstreamStrategy.GetClient().Delete(ctx, existing); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to delete stale envoypatchpolicy %s/%s: %w", existing.Namespace, existing.Name, err) + } + logger.Info("deleted stale envoypatchpolicy from downstream cluster", "namespace", existing.Namespace, "name", existing.Name) } - logger.Info("deleted stale envoypatchpolicy from downstream cluster", "namespace", existing.Namespace, "name", existing.Name) } if err := r.updateTPPAncestorsStatus(ctx, cl.GetClient(), trafficProtectionPolicies, originalTrafficProtectionPolicies); err != nil { @@ -1190,6 +1262,67 @@ func (r *TrafficProtectionPolicyReconciler) getDesiredEnvoyPatchPolicies( return desiredPolicies, nil } +func (r *TrafficProtectionPolicyReconciler) getDesiredExtendedSecurityPolicies( + downstreamNamespaceName string, + policyAttachments []policyAttachment, +) []*unstructured.Unstructured { + attachmentsByGateway := make(map[string][]policyAttachment, len(policyAttachments)) + gatewayKeys := make([]string, 0) + + for _, attachment := range policyAttachments { + // Only process gateway-level attachments; route-level is not supported by ESP. + if attachment.Route != nil { + continue + } + key := client.ObjectKeyFromObject(attachment.Gateway).String() + if _, ok := attachmentsByGateway[key]; !ok { + gatewayKeys = append(gatewayKeys, key) + } + attachmentsByGateway[key] = append(attachmentsByGateway[key], attachment) + } + + sort.Strings(gatewayKeys) + + desiredESPs := make([]*unstructured.Unstructured, 0, len(attachmentsByGateway)) + + for _, key := range gatewayKeys { + attachmentsForGateway := attachmentsByGateway[key] + if len(attachmentsForGateway) == 0 { + continue + } + + // Use the first (highest-priority) attachment's directives. + // Gateway-level attachments are already deduplicated (one per gateway) by + // collectTrafficProtectionPolicyAttachments. + attachment := attachmentsForGateway[0] + if len(attachment.CorazaDirectives) == 0 { + continue + } + + directives := strings.Join(attachment.CorazaDirectives, "\n") + + esp := newUnstructuredForGVK(tegESPGVK) + esp.SetName(tppEnvoyPatchPolicyPrefix + attachment.Gateway.Name) + esp.SetNamespace(downstreamNamespaceName) + esp.Object["spec"] = map[string]any{ + "targetRefs": []any{ + map[string]any{ + "group": gatewayv1.GroupName, + "kind": string(KindGateway), + "name": attachment.Gateway.Name, + }, + }, + "waf": map[string]any{ + "directives": directives, + }, + } + + desiredESPs = append(desiredESPs, esp) + } + + return desiredESPs +} + func getVHostConstraintForGateway(namespace string, gateway *gatewayv1.Gateway) string { return fmt.Sprintf(`@.kind=="%s" && @.namespace=="%s" && @.name=="%s"`, KindGateway, diff --git a/internal/controller/trafficprotectionpolicy_controller_test.go b/internal/controller/trafficprotectionpolicy_controller_test.go index 6f67857f..4b2549a1 100644 --- a/internal/controller/trafficprotectionpolicy_controller_test.go +++ b/internal/controller/trafficprotectionpolicy_controller_test.go @@ -762,7 +762,7 @@ func TestGetDesiredEnvoyPatchPolicies(t *testing.T) { var patchPolicy *envoygatewayv1alpha1.EnvoyPatchPolicy for _, p := range patchPolicies { - if p.Name == tppEnvoyPatchPolicyPrefix+attachment.Gateway.Name { + if p.Name == fmt.Sprintf("tpp-%s", attachment.Gateway.Name) { patchPolicy = p break } @@ -1463,6 +1463,273 @@ func newTestScheme() *runtime.Scheme { return testScheme } +func TestGetDesiredExtendedSecurityPolicies(t *testing.T) { + operatorConfig := config.NetworkServicesOperator{ + Gateway: config.GatewayConfig{ + TargetDomain: "example.com", + ListenerTLSOptions: map[gatewayv1.AnnotationKey]gatewayv1.AnnotationValue{ + gatewayv1.AnnotationKey("gateway.networking.datumapis.com/certificate-issuer"): gatewayv1.AnnotationValue("test"), + }, + Coraza: config.CorazaConfig{ + RouteBaseDirectives: []string{ + "Include @crs-setup-conf", "Include @recommended-conf", + }, + }, + }, + } + + reconciler := &TrafficProtectionPolicyReconciler{Config: operatorConfig} + + gateway1 := newGateway(operatorConfig, "default", "gateway-1") + gateway2 := newGateway(operatorConfig, "default", "gateway-2") + + downstreamNS := "test-downstream-ns" + + t.Run("basic gateway attachment produces one ESP", func(t *testing.T) { + policy := &policyContext{ + TrafficProtectionPolicy: ptr.To(newTrafficProtectionPolicy("default", "tpp-1")), + } + attachments := []policyAttachment{ + { + Policy: policy, + Gateway: gateway1, + CorazaDirectives: []string{"SecRuleEngine DetectionOnly"}, + }, + } + + esps := reconciler.getDesiredExtendedSecurityPolicies(downstreamNS, attachments) + + if assert.Len(t, esps, 1) { + esp := esps[0] + assert.Equal(t, tppEnvoyPatchPolicyPrefix+"gateway-1", esp.GetName()) + assert.Equal(t, downstreamNS, esp.GetNamespace()) + + spec, ok := esp.Object["spec"].(map[string]any) + if assert.True(t, ok, "spec must be a map") { + targetRefs, ok := spec["targetRefs"].([]any) + if assert.True(t, ok, "targetRefs must be a slice") && assert.Len(t, targetRefs, 1) { + ref, ok := targetRefs[0].(map[string]any) + if assert.True(t, ok) { + assert.Equal(t, "gateway-1", ref["name"]) + assert.Equal(t, string(KindGateway), ref["kind"]) + } + } + waf, ok := spec["waf"].(map[string]any) + if assert.True(t, ok) { + assert.Equal(t, "SecRuleEngine DetectionOnly", waf["directives"]) + } + } + } + }) + + t.Run("multiple gateways produce multiple ESPs", func(t *testing.T) { + policy := &policyContext{ + TrafficProtectionPolicy: ptr.To(newTrafficProtectionPolicy("default", "tpp-1")), + } + attachments := []policyAttachment{ + { + Policy: policy, + Gateway: gateway1, + CorazaDirectives: []string{"SecRuleEngine On"}, + }, + { + Policy: policy, + Gateway: gateway2, + CorazaDirectives: []string{"SecRuleEngine DetectionOnly"}, + }, + } + + esps := reconciler.getDesiredExtendedSecurityPolicies(downstreamNS, attachments) + assert.Len(t, esps, 2) + }) + + t.Run("mode Enforce yields SecRuleEngine On in directives", func(t *testing.T) { + policy := &policyContext{ + TrafficProtectionPolicy: ptr.To(newTrafficProtectionPolicy("default", "tpp-1", func(tpp *networkingv1alpha.TrafficProtectionPolicy) { + tpp.Spec.Mode = networkingv1alpha.TrafficProtectionPolicyEnforce + })), + } + directives := reconciler.getCorazaDirectivesForTrafficProtectionPolicy(policy) + attachments := []policyAttachment{ + {Policy: policy, Gateway: gateway1, CorazaDirectives: directives}, + } + esps := reconciler.getDesiredExtendedSecurityPolicies(downstreamNS, attachments) + + if assert.Len(t, esps, 1) { + spec := esps[0].Object["spec"].(map[string]any) + waf := spec["waf"].(map[string]any) + assert.Contains(t, waf["directives"], "SecRuleEngine On") + } + }) + + t.Run("mode Observe yields SecRuleEngine DetectionOnly in directives", func(t *testing.T) { + policy := &policyContext{ + TrafficProtectionPolicy: ptr.To(newTrafficProtectionPolicy("default", "tpp-1")), + } + directives := reconciler.getCorazaDirectivesForTrafficProtectionPolicy(policy) + attachments := []policyAttachment{ + {Policy: policy, Gateway: gateway1, CorazaDirectives: directives}, + } + esps := reconciler.getDesiredExtendedSecurityPolicies(downstreamNS, attachments) + + if assert.Len(t, esps, 1) { + spec := esps[0].Object["spec"].(map[string]any) + waf := spec["waf"].(map[string]any) + assert.Contains(t, waf["directives"], "SecRuleEngine DetectionOnly") + } + }) + + t.Run("mode Disabled yields SecRuleEngine Off in directives", func(t *testing.T) { + policy := &policyContext{ + TrafficProtectionPolicy: ptr.To(newTrafficProtectionPolicy("default", "tpp-1", func(tpp *networkingv1alpha.TrafficProtectionPolicy) { + tpp.Spec.Mode = networkingv1alpha.TrafficProtectionPolicyDisabled + })), + } + directives := reconciler.getCorazaDirectivesForTrafficProtectionPolicy(policy) + attachments := []policyAttachment{ + {Policy: policy, Gateway: gateway1, CorazaDirectives: directives}, + } + esps := reconciler.getDesiredExtendedSecurityPolicies(downstreamNS, attachments) + + if assert.Len(t, esps, 1) { + spec := esps[0].Object["spec"].(map[string]any) + waf := spec["waf"].(map[string]any) + assert.Contains(t, waf["directives"], "SecRuleEngine Off") + } + }) + + t.Run("paranoia levels and score thresholds appear in directives", func(t *testing.T) { + policy := &policyContext{ + TrafficProtectionPolicy: ptr.To(newTrafficProtectionPolicy("default", "tpp-1", func(tpp *networkingv1alpha.TrafficProtectionPolicy) { + owaspCRS := &tpp.Spec.RuleSets[0].OWASPCoreRuleSet + owaspCRS.ParanoiaLevels.Blocking = 3 + owaspCRS.ParanoiaLevels.Detection = 4 + owaspCRS.ScoreThresholds.Inbound = 100 + owaspCRS.ScoreThresholds.Outbound = 50 + })), + } + directives := reconciler.getCorazaDirectivesForTrafficProtectionPolicy(policy) + attachments := []policyAttachment{ + {Policy: policy, Gateway: gateway1, CorazaDirectives: directives}, + } + esps := reconciler.getDesiredExtendedSecurityPolicies(downstreamNS, attachments) + + if assert.Len(t, esps, 1) { + spec := esps[0].Object["spec"].(map[string]any) + waf := spec["waf"].(map[string]any) + directivesStr := waf["directives"].(string) + assert.Contains(t, directivesStr, "blocking_paranoia_level=3") + assert.Contains(t, directivesStr, "detection_paranoia_level=4") + assert.Contains(t, directivesStr, "inbound_anomaly_score_threshold=100") + assert.Contains(t, directivesStr, "outbound_anomaly_score_threshold=50") + } + }) + + t.Run("rule exclusions appear in directives", func(t *testing.T) { + policy := &policyContext{ + TrafficProtectionPolicy: ptr.To(newTrafficProtectionPolicy("default", "tpp-1", func(tpp *networkingv1alpha.TrafficProtectionPolicy) { + owaspCRS := &tpp.Spec.RuleSets[0].OWASPCoreRuleSet + owaspCRS.RuleExclusions = &networkingv1alpha.OWASPRuleExclusions{ + Tags: []networkingv1alpha.OWASPTag{"test-tag"}, + IDs: []int{12345}, + } + })), + } + directives := reconciler.getCorazaDirectivesForTrafficProtectionPolicy(policy) + attachments := []policyAttachment{ + {Policy: policy, Gateway: gateway1, CorazaDirectives: directives}, + } + esps := reconciler.getDesiredExtendedSecurityPolicies(downstreamNS, attachments) + + if assert.Len(t, esps, 1) { + spec := esps[0].Object["spec"].(map[string]any) + waf := spec["waf"].(map[string]any) + directivesStr := waf["directives"].(string) + assert.Contains(t, directivesStr, "SecRuleRemoveByTag") + assert.Contains(t, directivesStr, "test-tag") + assert.Contains(t, directivesStr, "SecRuleRemoveById 12345") + } + }) + + t.Run("route-level attachments are skipped", func(t *testing.T) { + policy := &policyContext{ + TrafficProtectionPolicy: ptr.To(newTrafficProtectionPolicy("default", "tpp-1")), + } + attachments := []policyAttachment{ + { + Policy: policy, + Gateway: gateway1, + Route: newHTTPRoute("default", "route-1"), + CorazaDirectives: []string{"SecRuleEngine DetectionOnly"}, + }, + } + + esps := reconciler.getDesiredExtendedSecurityPolicies(downstreamNS, attachments) + assert.Len(t, esps, 0, "route-level attachments must be skipped in TEG mode") + }) + + t.Run("ESP name is tpp-", func(t *testing.T) { + policy := &policyContext{ + TrafficProtectionPolicy: ptr.To(newTrafficProtectionPolicy("default", "tpp-1")), + } + attachments := []policyAttachment{ + { + Policy: policy, + Gateway: gateway1, + CorazaDirectives: []string{"SecRuleEngine On"}, + }, + } + + esps := reconciler.getDesiredExtendedSecurityPolicies(downstreamNS, attachments) + if assert.Len(t, esps, 1) { + assert.Equal(t, "tpp-gateway-1", esps[0].GetName()) + } + }) + + t.Run("ESP namespace is the downstream namespace", func(t *testing.T) { + policy := &policyContext{ + TrafficProtectionPolicy: ptr.To(newTrafficProtectionPolicy("default", "tpp-1")), + } + attachments := []policyAttachment{ + { + Policy: policy, + Gateway: gateway1, + CorazaDirectives: []string{"SecRuleEngine On"}, + }, + } + + esps := reconciler.getDesiredExtendedSecurityPolicies("my-downstream-ns", attachments) + if assert.Len(t, esps, 1) { + assert.Equal(t, "my-downstream-ns", esps[0].GetNamespace()) + } + }) + + t.Run("ESP targetRef points to the gateway by name", func(t *testing.T) { + policy := &policyContext{ + TrafficProtectionPolicy: ptr.To(newTrafficProtectionPolicy("default", "tpp-1")), + } + attachments := []policyAttachment{ + { + Policy: policy, + Gateway: gateway1, + CorazaDirectives: []string{"SecRuleEngine On"}, + }, + } + + esps := reconciler.getDesiredExtendedSecurityPolicies(downstreamNS, attachments) + if assert.Len(t, esps, 1) { + spec, ok := esps[0].Object["spec"].(map[string]any) + if assert.True(t, ok) { + targetRefs, ok := spec["targetRefs"].([]any) + if assert.True(t, ok) && assert.Len(t, targetRefs, 1) { + ref := targetRefs[0].(map[string]any) + assert.Equal(t, "gateway-1", ref["name"]) + } + } + } + }) +} + // TestTPPReconcileStaleCleanupPreservesConnectorEPPs is a regression test for the // bug where the TPP stale-cleanup loop deleted connector-* EnvoyPatchPolicies that // belong to the HTTPProxy controller. The stale cleanup must only delete EPPs whose @@ -1542,3 +1809,83 @@ func TestTPPReconcileStaleCleanupPreservesConnectorEPPs(t *testing.T) { err = fakeDownstreamClient.Get(ctx, client.ObjectKey{Name: "tpp-stale-gw", Namespace: downstreamNS}, stale) assert.True(t, apierrors.IsNotFound(err), "stale tpp EPP must be deleted by stale cleanup") } + +// TestTPPReconcileStaleCleanupTEGMode verifies that in TEG mode the stale-cleanup +// loop deletes ExtendedSecurityPolicy resources that carry tppManagedLabel but are +// no longer desired, while leaving ESPs that do not carry the label untouched. +func TestTPPReconcileStaleCleanupTEGMode(t *testing.T) { + // Upstream scheme: core (Namespace) + Gateway API + networking CRDs + upstreamScheme := runtime.NewScheme() + assert.NoError(t, scheme.AddToScheme(upstreamScheme)) + assert.NoError(t, gatewayv1.Install(upstreamScheme)) + assert.NoError(t, networkingv1alpha.AddToScheme(upstreamScheme)) + + const ( + upstreamNS = "default" + nsUID = "test-ns-uid-teg" + downstreamNS = "ns-" + nsUID + ) + + upstreamNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: upstreamNS, + UID: nsUID, + }, + } + fakeUpstreamClient := fake.NewClientBuilder(). + WithScheme(upstreamScheme). + WithObjects(upstreamNamespace). + Build() + + // Pre-populate downstream with two ESPs: + // tpp-stale-gw – managed by this controller (carries tppManagedLabel, must be deleted) + // unmanaged-esp – not managed by this controller (no label, must survive) + staleESP := &unstructured.Unstructured{} + staleESP.SetGroupVersionKind(tegESPGVK) + staleESP.SetName("tpp-stale-gw") + staleESP.SetNamespace(downstreamNS) + staleESP.SetLabels(map[string]string{tppManagedLabel: "true"}) + + unmanagedESP := &unstructured.Unstructured{} + unmanagedESP.SetGroupVersionKind(tegESPGVK) + unmanagedESP.SetName("unmanaged-esp") + unmanagedESP.SetNamespace(downstreamNS) + + // The downstream fake client needs no typed scheme for unstructured resources. + fakeDownstreamClient := fake.NewClientBuilder(). + WithObjects(staleESP, unmanagedESP). + Build() + + reconciler := &TrafficProtectionPolicyReconciler{ + mgr: &fakeMockManager{cl: fakeUpstreamClient}, + DownstreamCluster: &fakeCluster{cl: fakeDownstreamClient}, + Config: config.NetworkServicesOperator{ + Gateway: config.GatewayConfig{ + Coraza: config.CorazaConfig{ + Backend: config.WAFBackendTEGESP, + }, + }, + }, + } + + ctx := context.Background() + _, err := reconciler.Reconcile(ctx, NamespaceReconcileRequest{ + Namespace: upstreamNS, + ClusterName: "test-cluster", + }) + assert.NoError(t, err) + + // tpp-stale-gw must have been deleted. + gotStale := &unstructured.Unstructured{} + gotStale.SetGroupVersionKind(tegESPGVK) + err = fakeDownstreamClient.Get(ctx, client.ObjectKey{Name: "tpp-stale-gw", Namespace: downstreamNS}, gotStale) + assert.True(t, apierrors.IsNotFound(err), "stale managed ESP must be deleted by stale cleanup") + + // unmanaged-esp must survive — it has no tppManagedLabel. + gotUnmanaged := &unstructured.Unstructured{} + gotUnmanaged.SetGroupVersionKind(tegESPGVK) + assert.NoError(t, + fakeDownstreamClient.Get(ctx, client.ObjectKey{Name: "unmanaged-esp", Namespace: downstreamNS}, gotUnmanaged), + "unmanaged ESP must not be deleted", + ) +} From ebbc9d300a90027448bc793c915496a0131e069e Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Thu, 23 Apr 2026 14:25:20 -0700 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20address=20review=20issues=20and=20ad?= =?UTF-8?q?d=20EPP=E2=86=92ESP=20migration=20cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - add isTEGMode comment - fix error guard on ESP stale-cleanup List (drop !IsNotFound) - remove unreachable len==0 guard in getDesiredExtendedSecurityPolicies - fix misleading deduplication comment - introduce tppManagedLabelValue constant (goconst lint) - clean up legacy tpp-* EPPs when running in teg-esp mode (migration) - fix TestGetDesiredEnvoyPatchPolicies: use tppEnvoyPatchPolicyPrefix constant - fix TestTPPReconcileStaleCleanupTEGMode: register envoy scheme for migration - add TestTPPReconcileEPPToESPMigration covering coraza-epp → teg-esp switch --- .../trafficprotectionpolicy_controller.go | 40 ++++++--- ...trafficprotectionpolicy_controller_test.go | 89 ++++++++++++++++++- 2 files changed, 116 insertions(+), 13 deletions(-) diff --git a/internal/controller/trafficprotectionpolicy_controller.go b/internal/controller/trafficprotectionpolicy_controller.go index 681f82a2..8c562d28 100644 --- a/internal/controller/trafficprotectionpolicy_controller.go +++ b/internal/controller/trafficprotectionpolicy_controller.go @@ -71,6 +71,9 @@ const ( // instead of the current prefix check, removing the naming-convention // dependency entirely. tppManagedLabel = "networking.datumapis.com/managed-by-tpp-controller" + + // tppManagedLabelValue is the value stamped on tppManagedLabel. + tppManagedLabelValue = "true" ) // tegESPGVK is the GroupVersionKind for TEG's ExtendedSecurityPolicy CRD. @@ -94,6 +97,7 @@ type certificateReadinessResult struct { // +kubebuilder:rbac:groups=networking.datumapis.com,resources=trafficprotectionpolicies/finalizers,verbs=update // +kubebuilder:rbac:groups=cert-manager.io,resources=certificates,verbs=get;list;watch +// isTEGMode reports whether the WAF backend is TEG ExtendedSecurityPolicy. func (r *TrafficProtectionPolicyReconciler) isTEGMode() bool { return r.Config.Gateway.Coraza.Backend == config.WAFBackendTEGESP } @@ -168,7 +172,7 @@ func (r *TrafficProtectionPolicyReconciler) Reconcile(ctx context.Context, req N existing.SetLabels(make(map[string]string)) } labels := existing.GetLabels() - labels[tppManagedLabel] = "true" + labels[tppManagedLabel] = tppManagedLabelValue existing.SetLabels(labels) existing.Object["spec"] = esp.Object["spec"] return nil @@ -190,8 +194,8 @@ func (r *TrafficProtectionPolicyReconciler) Reconcile(ctx context.Context, req N ctx, existingESPList, client.InNamespace(downstreamNamespaceName), - client.MatchingLabels{tppManagedLabel: "true"}, - ); err != nil && !apierrors.IsNotFound(err) { + client.MatchingLabels{tppManagedLabel: tppManagedLabelValue}, + ); err != nil { return ctrl.Result{}, fmt.Errorf("failed to list extendedsecuritypolicies: %w", err) } for i := range existingESPList.Items { @@ -204,6 +208,27 @@ func (r *TrafficProtectionPolicyReconciler) Reconcile(ctx context.Context, req N } logger.Info("deleted stale extendedsecuritypolicy from downstream cluster", "namespace", existing.GetNamespace(), "name", existing.GetName()) } + + // Clean up any tpp-* EnvoyPatchPolicies left over from a previous coraza-epp + // deployment. This handles migration from coraza-epp to teg-esp. + var legacyEPPs envoygatewayv1alpha1.EnvoyPatchPolicyList + if err := downstreamStrategy.GetClient().List( + ctx, + &legacyEPPs, + client.InNamespace(downstreamNamespaceName), + ); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to list legacy envoypatchpolicies: %w", err) + } + for i := range legacyEPPs.Items { + epp := &legacyEPPs.Items[i] + if !strings.HasPrefix(epp.Name, tppEnvoyPatchPolicyPrefix) { + continue + } + if err := downstreamStrategy.GetClient().Delete(ctx, epp); err != nil && !apierrors.IsNotFound(err) { + return ctrl.Result{}, fmt.Errorf("failed to delete legacy envoypatchpolicy %s/%s: %w", epp.Namespace, epp.Name, err) + } + logger.Info("deleted legacy envoypatchpolicy during teg-esp migration", "namespace", epp.Namespace, "name", epp.Name) + } } else { // Coraza EPP mode (default): check cert and listener readiness before writing EPPs. @@ -257,7 +282,7 @@ func (r *TrafficProtectionPolicyReconciler) Reconcile(ctx context.Context, req N if policy.Labels == nil { policy.Labels = make(map[string]string) } - policy.Labels[tppManagedLabel] = "true" + policy.Labels[tppManagedLabel] = tppManagedLabelValue policy.Spec = desiredPolicy.Spec return nil }) @@ -1287,13 +1312,8 @@ func (r *TrafficProtectionPolicyReconciler) getDesiredExtendedSecurityPolicies( for _, key := range gatewayKeys { attachmentsForGateway := attachmentsByGateway[key] - if len(attachmentsForGateway) == 0 { - continue - } - // Use the first (highest-priority) attachment's directives. - // Gateway-level attachments are already deduplicated (one per gateway) by - // collectTrafficProtectionPolicyAttachments. + // Use the first (highest-priority) attachment's directives for this gateway. attachment := attachmentsForGateway[0] if len(attachment.CorazaDirectives) == 0 { continue diff --git a/internal/controller/trafficprotectionpolicy_controller_test.go b/internal/controller/trafficprotectionpolicy_controller_test.go index 4b2549a1..e35bc6fb 100644 --- a/internal/controller/trafficprotectionpolicy_controller_test.go +++ b/internal/controller/trafficprotectionpolicy_controller_test.go @@ -762,7 +762,7 @@ func TestGetDesiredEnvoyPatchPolicies(t *testing.T) { var patchPolicy *envoygatewayv1alpha1.EnvoyPatchPolicy for _, p := range patchPolicies { - if p.Name == fmt.Sprintf("tpp-%s", attachment.Gateway.Name) { + if p.Name == tppEnvoyPatchPolicyPrefix+attachment.Gateway.Name { patchPolicy = p break } @@ -1844,15 +1844,20 @@ func TestTPPReconcileStaleCleanupTEGMode(t *testing.T) { staleESP.SetGroupVersionKind(tegESPGVK) staleESP.SetName("tpp-stale-gw") staleESP.SetNamespace(downstreamNS) - staleESP.SetLabels(map[string]string{tppManagedLabel: "true"}) + staleESP.SetLabels(map[string]string{tppManagedLabel: tppManagedLabelValue}) unmanagedESP := &unstructured.Unstructured{} unmanagedESP.SetGroupVersionKind(tegESPGVK) unmanagedESP.SetName("unmanaged-esp") unmanagedESP.SetNamespace(downstreamNS) - // The downstream fake client needs no typed scheme for unstructured resources. + // The downstream client needs envoygateway scheme because TEG mode also runs + // the EPP migration cleanup which lists EnvoyPatchPolicies. + downstreamScheme := runtime.NewScheme() + assert.NoError(t, envoygatewayv1alpha1.AddToScheme(downstreamScheme)) + fakeDownstreamClient := fake.NewClientBuilder(). + WithScheme(downstreamScheme). WithObjects(staleESP, unmanagedESP). Build() @@ -1889,3 +1894,81 @@ func TestTPPReconcileStaleCleanupTEGMode(t *testing.T) { "unmanaged ESP must not be deleted", ) } + +// TestTPPReconcileEPPToESPMigration verifies that switching from coraza-epp to teg-esp +// causes any existing tpp-* EnvoyPatchPolicies to be deleted, while non-tpp EPPs +// (e.g. connector-*) are left untouched. +func TestTPPReconcileEPPToESPMigration(t *testing.T) { + upstreamScheme := runtime.NewScheme() + assert.NoError(t, scheme.AddToScheme(upstreamScheme)) + assert.NoError(t, gatewayv1.Install(upstreamScheme)) + assert.NoError(t, networkingv1alpha.AddToScheme(upstreamScheme)) + + downstreamScheme := runtime.NewScheme() + assert.NoError(t, envoygatewayv1alpha1.AddToScheme(downstreamScheme)) + + const ( + upstreamNS = "default" + nsUID = "test-ns-uid-migration" + downstreamNS = "ns-" + nsUID + ) + + upstreamNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: upstreamNS, + UID: nsUID, + }, + } + fakeUpstreamClient := fake.NewClientBuilder(). + WithScheme(upstreamScheme). + WithObjects(upstreamNamespace). + Build() + + // Legacy tpp-* EPP created by the coraza-epp backend. + legacyEPP := &envoygatewayv1alpha1.EnvoyPatchPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tpp-gateway-1", + Namespace: downstreamNS, + Labels: map[string]string{tppManagedLabel: tppManagedLabelValue}, + }, + } + // connector-* EPP that must NOT be deleted. + connectorEPP := &envoygatewayv1alpha1.EnvoyPatchPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "connector-tunnel-bar", Namespace: downstreamNS}, + } + fakeDownstreamClient := fake.NewClientBuilder(). + WithScheme(downstreamScheme). + WithObjects(legacyEPP, connectorEPP). + Build() + + reconciler := &TrafficProtectionPolicyReconciler{ + mgr: &fakeMockManager{cl: fakeUpstreamClient}, + DownstreamCluster: &fakeCluster{cl: fakeDownstreamClient}, + Config: config.NetworkServicesOperator{ + Gateway: config.GatewayConfig{ + Coraza: config.CorazaConfig{ + Backend: config.WAFBackendTEGESP, + }, + }, + }, + } + + ctx := context.Background() + _, err := reconciler.Reconcile(ctx, NamespaceReconcileRequest{ + Namespace: upstreamNS, + ClusterName: "test-cluster", + }) + assert.NoError(t, err) + + // Legacy tpp-* EPP must have been deleted during migration. + got := &envoygatewayv1alpha1.EnvoyPatchPolicy{} + err = fakeDownstreamClient.Get(ctx, client.ObjectKey{Name: "tpp-gateway-1", Namespace: downstreamNS}, got) + assert.True(t, apierrors.IsNotFound(err), "legacy tpp EPP must be deleted when migrating to teg-esp mode") + + // connector-* EPP must survive. + connector := &envoygatewayv1alpha1.EnvoyPatchPolicy{} + assert.NoError(t, + fakeDownstreamClient.Get(ctx, client.ObjectKey{Name: "connector-tunnel-bar", Namespace: downstreamNS}, connector), + "connector EPP must not be deleted by teg-esp migration cleanup", + ) +}