Hi @soneillf5

The implementation looks good.

However, I think we can make it more integrated with Kubernetes and add some usability. The issue I see is how the admin will propagate the cert and key to the file system of the pod? Also consider our three installation methods: manifests, helm chart and the operator.

A good approach that would work well for all installation methods is for the admin to create a Secret with a TLS cert and key and pass its name as an Ingress Controller parameter. This would be similar to the -default-server-tls-secret or -wildcard-tls-secret command-line flags.

Could we have a similar -prometheus-tls-secret flag?

If it is set, the TLS will be enabled for Prometheus metrics and the IC will use the TLS cert and key from the referenced Secret. If the secret doesn't exist or it is invalid, the IC will fail to start with an error.

An extra feature here - supporting updating that secret - if the Secret is updated, the IC will start using the new version. This is something that we support for the default and the wildcard secrets. However, supporting updating will be more involved, and we can start with a simple implementation without it.

Regarding different installation methods:

• For the manifests installation, the admin will need to create a TLS Secret and update the Ingress Controller deployment/daemonset to use it (the cli args). There are also annotations that are needed to be added as described here https://docs.nginx.com/nginx-ingress-controller/logging-and-monitoring/prometheus/#enabling-metrics
• For Helm chart, we can add parameters similar to the parameters for the default server secret and the wildcard secret ( see controller.wildcardTLS.*). There is no need to provide the default value of the cert and key. Note that we already have prometheus.* parameter, so it makes sense to extend them. For example, prometheus.tls.cert, prometheus.tls.key and prometheus.tls.secret. If the prometheus.tls.secret, helm will configure the IC args to use the referenced secret. If ``prometheus.tls.certandprometheus.tls.key` are set, helm will create a Secret with those values and configure the IC args to use it.
• For the Operator, we can add a field:

   prometheus:
     enable: true
     port: 9114
     tlsSecret: ns/name

Regarding documentation - we will need to document all new cli args, helm parameters, etc. It makes sense to mention that you can secure the Prometheus endpoint with TLS here https://docs.nginx.com/nginx-ingress-controller/logging-and-monitoring/prometheus/

Not tested, but we might need to add prometheus.io/scheme: https annotation to the pod template if TLS is enabled. For manifests installation, we can simply add and comment it out. For helm and operator, we will need to add it if the TLS is enabled for Prometheus.

While the operator lives in a separate repo and requires a separate PR, I mentioned it here for completeness.

@pleshakov I've updated to use a Kubernetes Secret resource for the TLS cert data, I still need to add an automated test for it, but I've confirmed it works locally. I'll update the operator/helm/docs in the next few days too.

Hi @soneillf5 the changes look good

There are few remaining comments:
#1652 (comment)
#1652 (comment)
#1652 (comment)