cec: support for explicit control of Cilium Envoy filter injection

[MitchLewis930]

User description

PR_045

---

PR Type

Enhancement

---

Description

• Add explicit annotation control for Cilium Envoy filter injection

• Introduce cec.cilium.io/inject-cilium-filters annotation for CiliumEnvoyConfig

• Decouple filter injection from L7LB service presence via new parameter

• Add comprehensive test coverage for injection behavior

---

Diagram Walkthrough

flowchart LR
  A["CiliumEnvoyConfig"] -->|"annotation: inject-cilium-filters"| B["injectCiliumEnvoyFilters()"]
  A -->|"Services defined"| B
  B -->|"true/false"| C["parseResources()"]
  C -->|"injectCiliumEnvoyFilters param"| D["Filter injection decision"]

Loading

File Walkthrough

	Relevant files
Configuration changes	k8s.go Add CEC annotation prefix and inject-cilium-filters key    pkg/annotation/k8s.go Add CECPrefix constant for CEC-related annotations Define CECInjectCiliumFilters annotation key Refactor CiliumPrefixRegex from var block to standalone variable +7/-4
Enhancement	cec_manager.go Implement filter injection logic and update resource parsing pkg/ciliumenvoyconfig/cec_manager.go Import annotation package for accessing CEC annotation constants Add injectCiliumEnvoyFilters() function to evaluate filter injection based on annotation or L7LB services Update all parseResources() calls to pass injection decision parameter Fix duplicate parameter in addCiliumEnvoyConfig() call +20/-1    cec_resource_parser.go Decouple filter injection from L7LB via new parameter        pkg/ciliumenvoyconfig/cec_resource_parser.go Add injectCiliumEnvoyFilters parameter to parseResources() function signature Replace isL7LB checks with injectCiliumEnvoyFilters for filter injection decisions Update documentation to clarify separation of L7LB metadata from filter injection Decouple filter injection logic from L7LB service presence +9/-8      exp_reflector.go Integrate filter injection into reflector logic                    pkg/ciliumenvoyconfig/exp_reflector.go Update cecListerWatchers() function signature formatting Add injectCiliumEnvoyFilters() call in registerCECReflector() function Pass injection decision to parseResources() method +3/-1
Tests	cec_manager_test.go Add injection logic tests and update test calls                    pkg/ciliumenvoyconfig/cec_manager_test.go Add comprehensive test suite Test_injectCiliumEnvoyFilters() with 6 test cases Test annotation override behavior, L7LB defaults, and invalid values Update existing test calls to parseResources() with new parameter Improve loop syntax using range instead of index +93/-3    cec_resource_parser_test.go Update tests for new filter injection parameter                    pkg/ciliumenvoyconfig/cec_resource_parser_test.go Update all parseResources() calls to include new injectCiliumEnvoyFilters parameter Add new test TestCiliumEnvoyConfigInjectCiliumFilters() validating filter non-injection Add test YAML configuration for filter injection scenarios Ensure backward compatibility with existing test expectations +79/-12

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Policy enforcement bypass Description: The new cec.cilium.io/inject-cilium-filters annotation allows explicitly disabling Cilium Envoy Network/L7 filter injection, which could weaken expected policy/enforcement behavior for a CiliumEnvoyConfig if untrusted principals are able to create/modify CECs (e.g., setting the annotation to false to avoid enforcement that would otherwise be applied when spec.services is present). cec_manager.go [599-612] Referred Code // injectCiliumEnvoyFilters returns true if the given object indicates that Cilium Envoy Network- and L7 filters // should be added to all non-internal Listeners. // This can be an explicit annotation or the implicit presence of a L7LB service via the Services property. func injectCiliumEnvoyFilters(meta *metav1.ObjectMeta, spec *ciliumv2.CiliumEnvoyConfigSpec) bool { if meta.GetAnnotations() != nil { if v, ok := meta.GetAnnotations()[annotation.CECInjectCiliumFilters]; ok { if boolValue, err := strconv.ParseBool(v); err == nil { return boolValue } } } return len(spec.Services) > 0 }
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Annotation ignored on add: addCiliumEnvoyConfig() passes len(cecSpec.Services) > 0 for both isL7LB and injectCiliumEnvoyFilters, preventing the new annotation-controlled behavior from taking effect during add operations. Referred Code resources, err := r.resourceParser.parseResources( cecObjectMeta.GetNamespace(), cecObjectMeta.GetName(), cecSpec.Resources, len(cecSpec.Services) > 0, len(cecSpec.Services) > 0, useOriginalSourceAddress(&cecObjectMeta), true, ) Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Misleading parameter docs: The new parameter documentation contains a typo (injecCiliumEnvoyFilters) that can confuse readers and makes the code less self-documenting. Referred Code // - `isL7LB` defines whether these resources are used for L7 loadbalancing. If `true`, the info gets passed to // the Envoy Cilium BPF Metadata listener filter on all Listeners. // - `injecCiliumEnvoyFilters` defines whether the Envoy Cilium Network- and L7 filters should always be added to all non-internal Listeners. // - `useOriginalSourceAddr` is passed to the Envoy Cilium BPF Metadata listener filter on all Listeners. // - `newResources` is passed as `true` when parsing resources that are being added or are the new version of the resources being updated, // and as `false` if the resources are being removed or are the old version of the resources being updated. Only 'new' resources are validated. func (r *cecResourceParser) parseResources(cecNamespace string, cecName string, xdsResources []cilium_v2.XDSResource, isL7LB bool, injectCiliumEnvoyFilters bool, useOriginalSourceAddr bool, newResources bool) (envoy.Resources, error) { Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Invalid annotation silent: Invalid values for the external annotation cec.cilium.io/inject-cilium-filters are silently ignored (no warning/metric), which may hinder detection of misconfiguration. Referred Code // injectCiliumEnvoyFilters returns true if the given object indicates that Cilium Envoy Network- and L7 filters // should be added to all non-internal Listeners. // This can be an explicit annotation or the implicit presence of a L7LB service via the Services property. func injectCiliumEnvoyFilters(meta *metav1.ObjectMeta, spec *ciliumv2.CiliumEnvoyConfigSpec) bool { if meta.GetAnnotations() != nil { if v, ok := meta.GetAnnotations()[annotation.CECInjectCiliumFilters]; ok { if boolValue, err := strconv.ParseBool(v); err == nil { return boolValue } } } return len(spec.Services) > 0 } Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix duplicated argument in function call Fix a duplicated argument in the r.resourceParser.parseResources call by replacing it with a call to injectCiliumEnvoyFilters to ensure the new annotation is respected. pkg/ciliumenvoyconfig/cec_manager.go [88-96] resources, err := r.resourceParser.parseResources( cecObjectMeta.GetNamespace(), cecObjectMeta.GetName(), cecSpec.Resources, len(cecSpec.Services) > 0, - len(cecSpec.Services) > 0, + injectCiliumEnvoyFilters(&cecObjectMeta, cecSpec), useOriginalSourceAddress(&cecObjectMeta), true, ) [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a significant bug where a duplicated argument was passed instead of the new logic, which would cause the feature to fail for new resources.	High
High-level	Decouple L7LB metadata from filter injection To improve clarity, rename the isL7LB parameter in parseResources to isL7LBMetadata, as its role is now limited to BPF metadata configuration. Examples: pkg/ciliumenvoyconfig/cec_resource_parser.go [128] func (r *cecResourceParser) parseResources(cecNamespace string, cecName string, xdsResources []cilium_v2.XDSResource, isL7LB bool, injectCiliumEnvoyFilters bool, useOriginalSourceAddr bool, newResources bool) (envoy.Resources, error) { Solution Walkthrough: Before: // in cec_resource_parser.go // The name `isL7LB` is now misleading as it only affects BPF metadata. func (r *cecResourceParser) parseResources( ..., isL7LB bool, injectCiliumEnvoyFilters bool, ... ) (envoy.Resources, error) { ... // `isL7LB` is only used here. if err := r.addCiliumBPFMetadataFilter(listener, isL7LB, ...); err != nil { ... } ... // Filter injection is controlled by the new parameter. injectCiliumDownstreamFilters := (... || injectCiliumEnvoyFilters) && ... ... } After: // in cec_resource_parser.go // The new name `isL7LBMetadata` accurately reflects its purpose. func (r *cecResourceParser) parseResources( ..., isL7LBMetadata bool, injectCiliumEnvoyFilters bool, ... ) (envoy.Resources, error) { ... // The usage is now self-documenting. if err := r.addCiliumBPFMetadataFilter(listener, isL7LBMetadata, ...); err != nil { ... } ... // Filter injection logic remains unchanged. injectCiliumDownstreamFilters := (... || injectCiliumEnvoyFilters) && ... ... } Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that the role of isL7LB has been reduced, and renaming it would improve clarity and complete the conceptual decoupling introduced in the PR.	Low
General	Simplify logic by removing redundant check Simplify the injectCiliumEnvoyFilters function by removing the redundant meta.GetAnnotations() != nil check, as accessing a key on a nil map is safe in Go. pkg/ciliumenvoyconfig/cec_manager.go [599-612] func injectCiliumEnvoyFilters(meta *metav1.ObjectMeta, spec *ciliumv2.CiliumEnvoyConfigSpec) bool { - if meta.GetAnnotations() != nil { - if v, ok := meta.GetAnnotations()[annotation.CECInjectCiliumFilters]; ok { - if boolValue, err := strconv.ParseBool(v); err == nil { - return boolValue - } + if v, ok := meta.GetAnnotations()[annotation.CECInjectCiliumFilters]; ok { + if boolValue, err := strconv.ParseBool(v); err == nil { + return boolValue } } return len(spec.Services) > 0 } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 4 __ Why: The suggestion correctly points out that the nil check is redundant in Go and proposes a valid simplification, which improves code conciseness and follows idiomatic Go practices.	Low
	Fix doc parameter name typo Fix a typo in the documentation comment for the parseResources function, changing injecCiliumEnvoyFilters to injectCiliumEnvoyFilters. pkg/ciliumenvoyconfig/cec_resource_parser.go [124] -// - `injecCiliumEnvoyFilters` defines whether the Envoy Cilium Network- and L7 filters should always be added to all non-internal Listeners. +// - `injectCiliumEnvoyFilters` defines whether the Envoy Cilium Network- and L7 filters should always be added to all non-internal Listeners. Apply / Chat Suggestion importance[1-10]: 2 __ Why: The suggestion correctly identifies and fixes a typo in a code comment, which improves documentation clarity and maintainability.	Low
More