feat(filter): make filter Service type configurable (ClusterIP/LoadBalancer/NodePort)

[nikitos]

Summary

Adds an optional type field to ServicePort, letting each exposed filter port choose its Kubernetes Service type. Defaults to ClusterIP, with LoadBalancer and NodePort available for clusters that need external exposure.

What changed

• api/v1alpha1/pipeline_types.go: new optional Type corev1.ServiceType field on ServicePort, default ClusterIP, enum ClusterIP;LoadBalancer;NodePort.
• internal/controller/pipelineinstance_controller_streaming.go: ensureFilterServices now sets the Service type from svcPort.Type, falling back to ClusterIP when unset (previously hardcoded).
• config/crd/bases and deployment/.../crds: regenerated CRDs with the new type property and default.

Testing

Unit tests for ensureFilterServices: type unset defaults to ClusterIP, explicit ClusterIP, LoadBalancer, NodePort, empty service list, update of an existing Service's type, and multiple ports per filter.

Notes

Selecting LoadBalancer provisions one external load balancer per port.

[lucasmundim]

Thanks for the contribution and for iterating quickly on the validation fixes — the enum now produces real ClusterIP/LoadBalancer values and the default is a valid ServiceType, so the PR is functional now. Nice catch on regenerating the CRD YAMLs in both locations.

This composes well with the ServicePort design discussion in #64 (where we're proposing an appProtocol field for the OpenFilter ZMQ dual-port case). Whichever direction that lands, type is orthogonal and stands on its own — we can land this independently once tests are in.

Small remaining asks — see inline. Summary: add tests for the new field, fix a docstring nit, and a question about whether to also include NodePort.

[lucasmundim]

Thanks for the thorough follow-up — the docstring reads well, NodePort is in the enum, and the test coverage goes beyond what was asked: the update-existing-Service and multiple-services-per-filter cases are particularly nice to have. The defense-in-depth ClusterIP fallback in the controller is also a good touch. Looks good to merge.

[shingonoide]

Thanks for the contribution, @nikitos. The per-port service type is a useful addition and the test coverage for the three values is thorough.

[concern] Updating an existing Service via a full Spec assignment can drop server-assigned fields. In the update path (pipelineinstance_controller_streaming.go:663-669), service.Spec = desiredService.Spec then a MergeFrom patch replaces the whole spec with one built without ClusterIP/ClusterIPs. The patch sends an empty clusterIP, which the API server rejects ("primary clusterIP can not be unset") for an existing ClusterIP Service. (A type change itself, for example ClusterIP to LoadBalancer, is allowed; the issue is the cleared ClusterIP, not type immutability.) The fake client in TestEnsureFilterServices_UpdateExistingService does not enforce this, so the test passes while a real update would error. Patching only the intended fields, or carrying over ClusterIP/ClusterIPs from the live object, avoids it; an envtest-based test would catch this class automatically.

[nit] makeReconcilerWithFakeClient mutates the global scheme.Scheme (..._streaming_test.go:929-944); other tests use a fresh runtime.NewScheme(). Idempotent today, but the local-scheme pattern is more consistent.

[nit] ExternalName omission is undocumented (pipeline_types.go:165); a one-line note would show it is deliberate.

[nit] Type doc comment is one long line (pipeline_types.go:162) while other fields wrap.

What works well: the default marker plus the controller-side serviceType == "" fallback cover both paths; both CRD manifests stay in sync; the multi-service test guards the dual-port pattern.