Helm chart: Expose Service Insight port in Service resource when feature is enabled

[aknot242]

Is your feature request related to a problem? Please describe.
When the Service Insight feature is enabled, the port is exposed in the NIC container. However, unless this port is exposed in the Service resource, it cannot be used by external services.

Describe the solution you'd like
If the serviceInsight.create Helm value is set to true, create additional ports for service insights as are currently done for the http and https services. Optionally, I propose that we allow the user to specify a NodePort for this service, which would require the addition of additional Helm parameters for serviceInsight of type and nodePort.

Describe alternatives you've considered
In an environment where a user can update the services post-deployment they could add their own Service resource, but this won't work in "self-healing" environments that detect and correct configuration drift. This is best implemented in the Helm chart.

Additional context
Since serviceInsight's initial implementation did not contain a service map as controller does, it is possible that this solution will add additional uniformity issues, or may require breaking changes to the current parameter object model.

[github-actions]

Hi @aknot242 thanks for reporting!

Be sure to check out the docs and the Contributing Guidelines while you wait for a human to take a look at this 🙂

Cheers!

[brianehlert]

I would like to hear from others.
my concern is the downstream impacts of this resulting in someone's Deep Service Insight endpoint being exposed publicly, because of an object such as loadbalancer blindly publishing it because it is automatically publishing all of the exposed NIC Service ports.
I could see the same thing happen if the same behavior was enabled for the Prometheus port.

[coolbry95]

I could see the same thing happen if the same behavior was enabled for the Prometheus port.

For prometheus I think it would be best to have another service. It might make sense for this as well. They could be on the same service. This is not part of the LB service but could just be a cluster type service.

[aknot242]

This may not be needed after all. It seems someone was thinking ahead and exposed controller.service.customPorts as a helm parameter. I'll give this a go.

controller:
    service:
      customPorts:
        - port: 9114
          targetPort: service-insight
          protocol: TCP
          name: service-insight

[AlexFenlon]

Hi @aknot242 ,

Have you had a chance to try the solution you commented above?

[aknot242]

@AlexFenlon sorry, it has been so long now, I can't recall.

[vepatel]

@aknot242 we're closing this now, please open a new issue or proposal based on the requirements.