Skip to content

Comments

Add metric to monitor cert expiry time#2592

Closed
annanay25 wants to merge 2 commits intocortexproject:masterfrom
annanay25:cert-monitor
Closed

Add metric to monitor cert expiry time#2592
annanay25 wants to merge 2 commits intocortexproject:masterfrom
annanay25:cert-monitor

Conversation

@annanay25
Copy link
Contributor

@annanay25 annanay25 commented May 14, 2020

What this PR does:
Adds metric to monitor TLS client cert expiry time.

Which issue(s) this PR fixes:
Follow up from #2502

Checklist

  • NA Tests updated
  • NA Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: Annanay annanayagarwal@gmail.com

Signed-off-by: Annanay <annanayagarwal@gmail.com>
Signed-off-by: Annanay <annanayagarwal@gmail.com>
@pstibrany
Copy link
Contributor

There are tools for this already, why have it in Cortex?

caCertPool = x509.NewCertPool()
caCertPool.AppendCertsFromPEM(caCert)
if len(clientCert.Certificate) > 0 && caCertPool != nil {
prometheus.MustRegister(tlsCertNotAfterTimestamp)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use new promauto package and give it a registrerer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not even correct register it here, because if the GetTLSConfig() is called twice, it will panic because of a double registration. You can just use promauto.NewGaugeVec() above, even if we should stop using the global prometheus registerer, but if you really want this metric then using the global registry is the easiest option given the current design.

@jtlisi
Copy link
Contributor

jtlisi commented Jul 1, 2020

I'm closing this for now since the general consensus is an external tool should be used to monitor this.

@jtlisi jtlisi closed this Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants