Helm chart: add ability to add appprotocol to port in service

[KipsasJaujoj]

Is your feature request related to a problem? Please describe. When setting up prometheus to scrape metrics using Istio mTLS (guide here: https://istio.io/latest/docs/ops/integrations/prometheus/#tls-settings) cert-manager scraping breaks due to it using tcp-prometheus-servicemonitor as a port name and suggesting istio tcp` protocol should be used for connection. This can be fixed either:

• changing service's port name to http-prometheus-servicemonitor
• adding appProtocol: http attribute to service's port

Describe the solution you'd like For me, any of the options will work since they both solve the issue in the same manner. Although, it's worth mentioning that appProtocol was introduced in kubernetes version 1.20 so not everyone will be able to use the solution. Maybe using the latter option (appProtocol) is a bit more clear on intent as ability to change port name via helm values might be a bit ambiguous feature.

Describe alternatives you've considered Alternatives are:

• not use Istio
• not use mTLS for scraping Not viable options for me, I'll have to fork the helm chart and add the feature myself

Additional context Following prometheus issue helped me find the solution: https://github.com/prometheus/prometheus/issues/10213#issuecomment-1064280828 More info on auto protocol selection: https://istio.io/latest/docs/ops/configuration/traffic-management/protocol-selection/#explicit-protocol-selection

Environment details (remove if not applicable):

• Install method: helm

/kind feature

[SgtCoDFish]

Hey, thanks for raising this! All of our supported releases target k8s 1.22 or newer so a requirement on k8s 1.20 won't be a problem for a change like this.

We'd definitely appreciate a PR for this. Top tip is to post a PR in #cert-manager-dev on k8s slack to get a maintainer's attention quickly.

[cert-manager-bot]

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. If this issue is safe to close now please do so with /close. /lifecycle stale

[KipsasJaujoj]

/remove-lifecycle stale

[cert-manager-bot]

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. If this issue is safe to close now please do so with /close. /lifecycle stale

[cert-manager-bot]

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity. If this issue is safe to close now please do so with /close. /lifecycle rotten /remove-lifecycle stale

[andriisoldatenko]

@KipsasJaujoj can you please share more details how can we test change from #7652 ?

I've tried to install cert-manager helm chart into istio-injection=enabled namespace, and tried different things and checked with and without my change:

k get svc cert-manager -n cert-manager -o yaml | yq .spec
clusterIP: 34.118.226.49
clusterIPs:
  - 34.118.226.49
internalTrafficPolicy: Cluster
ipFamilies:
  - IPv4
ipFamilyPolicy: SingleStack
ports:
  - name: tcp-prometheus-servicemonitor
    port: 9402
    protocol: TCP
    targetPort: 9402
selector:
  app.kubernetes.io/component: controller
  app.kubernetes.io/instance: cert-manager
  app.kubernetes.io/name: cert-manager
sessionAffinity: None
type: ClusterIP

there is no big difference, except what's document inside istio docs:

This can be configured in two ways: By the name of the port: name: [-]. In Kubernetes 1.18+, by the appProtocol field: appProtocol: .

I'm wondering if there is another easy way to test, instead of install prometheus, mTLS and istio.

so would be great if you help to test, or explain more details how can we test it. What kind of istio feature you are using.

[cert-manager-bot]

Rotten issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle rotten. /close

[cert-manager-prow[bot]]

@cert-manager-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle rotten. /close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[KipsasJaujoj]

/reopen

[cert-manager-prow[bot]]

@KipsasJaujoj: Reopened this issue.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[KipsasJaujoj]

@andriisoldatenko sure, I can help you testing this. As long as appProtocol: http will be present on k get svc cert-manager -n cert-manager -o yaml | yq .spec - that will solve the issue.

[KipsasJaujoj]

/remove-lifecycle rotten

[KipsasJaujoj]

@andriisoldatenko, sorry for the late response. My use case is covered and it works just fine. But as a person commented on the PR: if tls is enabled on metrics, this should be set to https to avoid issues.

[KipsasJaujoj]

I've tested mTLS scrape without your fix and it works as well. So it got fixed in some release. I've tested on istio v1.24.0 version. Issue opened when we had 1.21.4 version.