Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Confusing namespace label in SSL metrcis #11191

Closed
Riohyc opened this issue Apr 2, 2024 · 4 comments
Closed

Confusing namespace label in SSL metrcis #11191

Riohyc opened this issue Apr 2, 2024 · 4 comments
Labels
needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. triage/needs-information Indicates an issue needs more information in order to work on it.

Comments

@Riohyc
Copy link

Riohyc commented Apr 2, 2024

What happened:
nginx_ingress_controller_ssl_certificate_info and nginx_ingress_controller_ssl_expire_time_seconds are two ssl related metrics. For example, I have a tls secret gateway-perf-resource/wildcard-tls and ingress-nginx is deployed in gateway-perf

nginx_ingress_controller_ssl_certificate_info{class="lcap.com/gateway-perf",host="_",identifier="-10606596870633663195",issuer_common_name="ingress-test.com",issuer_organization="",namespace="gateway-perf-resources",public_key_algorithm="RSA",secret_name="wildcard-tls",serial_number="10606596870633663195"} 1
nginx_ingress_controller_ssl_expire_time_seconds{class="lcap.com/gateway-perf",host="_",namespace="gateway-perf",secret_name="wildcard-tls"} 1.742623556e+09

Both metrics aim to same secret. But

  • For namespace label in nginx_ingress_controller_ssl_certificate_info, it is the namespace of tls secret itself. Because it is overwrited by
    func (cm *Controller) SetSSLInfo(servers []*ingress.Server) {
    for _, s := range servers {
    if s.SSLCert == nil || s.SSLCert.Certificate == nil || s.SSLCert.Certificate.SerialNumber == nil {
    continue
    }
    labels := make(prometheus.Labels, len(cm.labels)+1)
    for k, v := range cm.labels {
    labels[k] = v
    }
    labels["identifier"] = s.SSLCert.Identifier()
    labels["host"] = s.Hostname
    labels["secret_name"] = s.SSLCert.Name
    labels["namespace"] = s.SSLCert.Namespace
  • For namespace label in nginx_ingress_controller_ssl_expire_time_seconds, it is the namespace of ingress-nginx-controller which is populated by
    cm := &Controller{
    constLabels: constLabels,
    labels: prometheus.Labels{
    "namespace": namespace,

What you expected to happen:

  • namepace should has same meaning in those two metrics.
  • add controller_namespace if needed

NGINX Ingress controller version (exec into the pod and run nginx-ingress-controller --version.):
Not releated, looks like nothing change in those metrics part

-------------------------------------------------------------------------------
NGINX Ingress controller
  Release:       v1.2.0
  Build:         a2514768cd282c41f39ab06bda17efefc4bd233a
  Repository:    https://github.com/kubernetes/ingress-nginx
  nginx version: nginx/1.19.10

-------------------------------------------------------------------------------

Kubernetes version (use kubectl version):
1.19.12
Environment:
Not related

How to reproduce this issue:
Any tls secret has different namespace with ingress-nginx-controller itself. e.g.

  1. deploy ingress-nginx-controller in namespace-a
  2. create a tls secret in namespace-b
  3. use --default-ssl-certificate=namespace-b/some-secret
  4. curl <ip>:10254/metrics | grep ssl to get the metrics

Anything else we need to know:
N/A

@Riohyc Riohyc added the kind/bug Categorizes issue or PR as related to a bug. label Apr 2, 2024
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels Apr 2, 2024
@longwuyuan
Copy link
Contributor

/remove-kind bug

  • You are using too old version of controller. Please change to v1.10 of controller
  • There is a PR to remove the cert related metrics
  • I am not convinced if both namespace is same
  • Nobody doing maintenance of that part of code but maybe it was designed to show expire time of all certs that one controller knows, in a cluster that has multiple instances of the controller
  • Wait for other comments

/triage needs-information

@k8s-ci-robot k8s-ci-robot added triage/needs-information Indicates an issue needs more information in order to work on it. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. and removed kind/bug Categorizes issue or PR as related to a bug. labels Apr 2, 2024
@Riohyc
Copy link
Author

Riohyc commented Apr 2, 2024

Thanks for reply.
Could you link the PR that remove the cert related metrics?

@longwuyuan
Copy link
Contributor

I see stalled #9706

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. triage/needs-information Indicates an issue needs more information in order to work on it.
Projects
Development

No branches or pull requests

3 participants