Add per-capability Service definition with NetworkPolicy integration - #16
Open
chrisboyd-kentik wants to merge 4 commits into
Open
Add per-capability Service definition with NetworkPolicy integration#16chrisboyd-kentik wants to merge 4 commits into
chrisboyd-kentik wants to merge 4 commits into
Conversation
- Add service.yaml template with per-capability port toggles (flowProxy, snmpTrap, syslog, bgp, synthetics, healthCheck) - Add nodePort support for NodePort/LoadBalancer service types - Add containerPort declarations decoupled from service.enabled - Add validation: fail template when service.enabled but no capabilities enabled - Add automatic NetworkPolicy ingress rules for enabled service ports - Add automatic NetworkPolicy egress for synthetics TCP 8877 - Add commented egress rules for SNMP/ST polling (ranger) and SSH (NMS) - Add values.schema.json with additionalProperties enforcement - Add 13 helm unit tests for service template - Rewrite README with service configuration front and center - Add .DS_Store to .gitignore
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in Kubernetes Service to the kagent Helm chart that exposes inbound ports on a per-capability basis, and extends the chart’s NetworkPolicy template to automatically align ingress/egress rules with the enabled capability ports.
Changes:
- Introduces
templates/service.yamlto render a Service with conditional ports per UA capability (including NodePort support). - Adds service-related helpers/validation and renders capability
containerPortsindependently ofservice.enabled. - Updates
templates/networkpolicy.yaml, values documentation, JSON schema, and adds helm-unittest coverage for Service rendering and validation.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| values.yaml | Adds service configuration block + clarifies NetworkPolicy comments and capability notes |
| templates/service.yaml | New Service template with per-capability conditional port rendering |
| templates/_helpers.tpl | Adds service validation, hasServicePorts, and capability-driven containerPorts rendering |
| templates/networkpolicy.yaml | Adds ingress generation for enabled capability ports and synthetics TCP/8877 egress |
| contracts/values.schema.json | Defines full service schema with additionalProperties: false and per-capability objects |
| tests/service_test.yaml | New unit tests covering Service rendering across capability combinations and options |
| tests/validation_test.yaml | Adds a validation test for service.enabled with no capability ports enabled |
| README.md | Rewrites documentation to center service capabilities and NetworkPolicy integration |
| .gitignore | Ignores .DS_Store |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix empty egress list producing invalid YAML (guard toYaml with if) - Fix nodePort rendering with ClusterIP type (only render for NodePort/LoadBalancer) - Auto-add probe port to NetworkPolicy ingress when probes are enabled - Document synthetics egress allows all destinations (by design for mesh tests) - Add NetworkPolicy test suite (12 tests covering ingress/egress/probes/edge cases) - Add ClusterIP nodePort exclusion test to service suite
|
It would be nice to see support for IPv6/dual stack services and also externalTrafficPolicy customization. ie: Specifically externalTrafficPolicy local is required to maintain the integrity of the IP header through to the receiving pod. Normally internal cluster networking will overwrite this header and along with it the src IP address of the sender. This is a major problem for something like netflow/ipfix where the identity of the sender (often) is that src IP address and there are no other options. |
- Add service.externalTrafficPolicy (Local/Cluster) to preserve source IP for flow receivers - Add service.ipFamilyPolicy and service.ipFamilies for IPv6/dual-stack services - Update values.schema.json with new fields - Add 4 unit tests covering externalTrafficPolicy and dual-stack rendering
…rational improvements Phase 1: Headless Service + sessionAffinity - Create headless-service.yaml for StatefulSet DNS resolution - Add sessionAffinity and sessionAffinityConfig to Service spec Phase 2: loadBalancerSourceRanges + independent targetPort - Add loadBalancerSourceRanges to restrict LB source IPs - Add per-capability targetPort field (container port can differ from service port) - Update containerPorts and NetworkPolicy to use targetPort Phase 3: healthCheck auto-enable - Auto-enable K_HC_SERVER_ENABLED when service.healthCheck.enabled is true - Documented override via configmap Phase 4: NetworkPolicy ingressFrom selector - Add optional networkPolicy.ingressFrom to restrict ingress sources - Default behavior unchanged (all sources allowed when not set) Phase 5: Documentation - Added port-matching warnings per capability (portal vs chart) - Expanded externalTrafficPolicy warning for single-replica - Added .gitignore entries for test artifacts Phase 6: Tests (65 total, up from 55) - Headless service tests (statefulset renders, daemonset skips) - sessionAffinity, loadBalancerSourceRanges, targetPort tests - ingressFrom selector tests - HC auto-enable test - targetPort in NetworkPolicy ingress rules test Live-validated on k3s v1.36.2 - all features confirmed working.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a Kubernetes Service resource to the kagent Helm chart with per-capability port toggles. Each UA capability that receives inbound traffic can be independently enabled, and the NetworkPolicy is automatically updated with matching ingress/egress rules.
Changes
templates/service.yaml- New Service template with conditional port rendering per capabilitytemplates/_helpers.tpl-containerPorts(decoupled from service.enabled),hasServicePortshelper, service validation (fails if enabled with no capabilities)templates/networkpolicy.yaml- Auto-generates Ingress rules for enabled service ports and egress for synthetics TCP 8877values.yaml- Service config block with per-capability toggles, nodePort support, ranger/SNMP/SSH egress commentscontracts/values.schema.json- Full schema withadditionalProperties: falseon all service objectstests/service_test.yaml- 13 unit tests covering all capabilities, custom ports, nodePort, annotations, service typestests/validation_test.yaml- Added empty-ports validation testREADME.md- Rewritten (929 → ~270 lines) with service configuration front and center.gitignore- Added.DS_StoreService Capabilities
flowProxysnmpTrapsyslogbgpsyntheticshealthCheckTesting
helm lintpasses with all combinationshelm unittest- 39/39 tests pass