security: cache certificate expiration metrics as pointers

Changes in #130110 were added to add labelled ttl metrics to
client certificates. It achieved this by changing the system
which cached certificate expiries to cache on a composite
struct of two metrics, rather than just an expiration
metric.

The struct itself housed the metrics as inline values,
rather than pointers, so updates were registered in the
cached values only, and not the registry in which they were
reporting. This means that updates to client certificate
expirations would not be reflected by the ttl or expiration
metrics.

This ticket modifies those elements so that they are not
copied when they are pulled from the cache.

Fixes: #142681
Epic: CRDB-40209

Release note (bug fix): Fixes bug in client certificate expiration metrics.

Author: angles-n-daemons

pkg/security/BUILD.bazel

@@ -107,82 +107,68 @@ go_test(
         "//pkg/util/uuid",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_go_ldap_ldap_v3//:ldap",
+        "@com_github_prometheus_client_model//go",
         "@com_github_stretchr_testify//require",
         "@org_golang_x_exp//rand",
     ] + select({
         "@io_bazel_rules_go//go/platform:aix": [
             "//pkg/util/log/eventpb",
-            "@com_github_prometheus_client_model//go",
             "@org_golang_x_sys//unix",
         ],
         "@io_bazel_rules_go//go/platform:android": [
             "//pkg/util/log/eventpb",
-            "@com_github_prometheus_client_model//go",
             "@org_golang_x_sys//unix",
         ],
         "@io_bazel_rules_go//go/platform:darwin": [
             "//pkg/util/log/eventpb",
-            "@com_github_prometheus_client_model//go",
             "@org_golang_x_sys//unix",
         ],
         "@io_bazel_rules_go//go/platform:dragonfly": [
             "//pkg/util/log/eventpb",
-            "@com_github_prometheus_client_model//go",
             "@org_golang_x_sys//unix",
         ],
         "@io_bazel_rules_go//go/platform:freebsd": [
             "//pkg/util/log/eventpb",
-            "@com_github_prometheus_client_model//go",
             "@org_golang_x_sys//unix",
         ],
         "@io_bazel_rules_go//go/platform:illumos": [
             "//pkg/util/log/eventpb",
-            "@com_github_prometheus_client_model//go",
             "@org_golang_x_sys//unix",
         ],
         "@io_bazel_rules_go//go/platform:ios": [
             "//pkg/util/log/eventpb",
-            "@com_github_prometheus_client_model//go",
             "@org_golang_x_sys//unix",
         ],
         "@io_bazel_rules_go//go/platform:js": [
             "//pkg/util/log/eventpb",
-            "@com_github_prometheus_client_model//go",
             "@org_golang_x_sys//unix",
         ],
         "@io_bazel_rules_go//go/platform:linux": [
             "//pkg/util/log/eventpb",
-            "@com_github_prometheus_client_model//go",
             "@org_golang_x_sys//unix",
         ],
         "@io_bazel_rules_go//go/platform:netbsd": [
             "//pkg/util/log/eventpb",
-            "@com_github_prometheus_client_model//go",
             "@org_golang_x_sys//unix",
         ],
         "@io_bazel_rules_go//go/platform:openbsd": [
             "//pkg/util/log/eventpb",
-            "@com_github_prometheus_client_model//go",
             "@org_golang_x_sys//unix",
         ],
         "@io_bazel_rules_go//go/platform:osx": [
             "//pkg/util/log/eventpb",
-            "@com_github_prometheus_client_model//go",
             "@org_golang_x_sys//unix",
         ],
         "@io_bazel_rules_go//go/platform:plan9": [
             "//pkg/util/log/eventpb",
-            "@com_github_prometheus_client_model//go",
             "@org_golang_x_sys//unix",
         ],
         "@io_bazel_rules_go//go/platform:qnx": [
             "//pkg/util/log/eventpb",
-            "@com_github_prometheus_client_model//go",
             "@org_golang_x_sys//unix",
         ],
         "@io_bazel_rules_go//go/platform:solaris": [
             "//pkg/util/log/eventpb",
-            "@com_github_prometheus_client_model//go",
             "@org_golang_x_sys//unix",
         ],
         "//conditions:default": [],

pkg/security/cert_expiry_cache.go

@@ -35,8 +35,8 @@ var ClientCertExpirationCacheCapacity = settings.RegisterIntSetting(
 	settings.WithPublic)
 
 type clientCertExpirationMetrics struct {
-	expiration aggmetric.Gauge
-	ttl        aggmetric.Gauge
+	expiration *aggmetric.Gauge
+	ttl        *aggmetric.Gauge
 }
 
 // ClientCertExpirationCache contains a cache of gauge objects keyed by
@@ -189,7 +189,7 @@ func (c *ClientCertExpirationCache) MaybeUpsert(
 				expiration := parentExpirationGauge.AddChild(key)
 				expiration.Update(newExpiry)
 				ttl := parentTTLGauge.AddFunctionalChild(ttlFunc(c.timeNow, newExpiry), key)
-				c.mu.cache.Add(key, &clientCertExpirationMetrics{*expiration, *ttl})
+				c.mu.cache.Add(key, &clientCertExpirationMetrics{expiration, ttl})
 			}
 		} else {
 			log.Ops.Warningf(ctx, "no memory available to cache cert expiry: %v", err)

pkg/security/cert_expiry_cache_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/mon"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
+	io_prometheus_client "github.com/prometheus/client_model/go"
 	"github.com/stretchr/testify/require"
 )
 
@@ -120,6 +121,55 @@ func TestEntryCache(t *testing.T) {
 	require.Equal(t, laterExpiration-(closerExpiration+20), ttl)
 }
 
+// TestCacheMetricsSync verifies that the cache metrics are correctly synchronized
+// when entries are inserted and updated. It checks that the cache length and
+// expiration times are properly updated and reflected in the metrics.
+func TestCacheMetricsSync(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+
+	findChildMetric := func(metrics *aggmetric.AggGauge, childName string) *io_prometheus_client.Metric {
+		var result *io_prometheus_client.Metric
+		metrics.Each([]*io_prometheus_client.LabelPair{}, func(metric *io_prometheus_client.Metric) {
+			if metric.GetLabel()[0].GetValue() == childName {
+				result = metric
+			}
+		})
+		return result
+	}
+
+	const (
+		fooUser = "foo"
+
+		laterExpiration  = int64(1684359292)
+		closerExpiration = int64(1584359292)
+	)
+
+	ctx := context.Background()
+
+	timesource := timeutil.NewManualTime(timeutil.Unix(0, 123))
+	// Create a cache with a capacity of 3.
+	cache, expMetric, ttlMetric := newCache(
+		ctx,
+		&cluster.Settings{},
+		3, /* capacity */
+		timesource,
+	)
+	require.Equal(t, 0, cache.Len())
+
+	// insert.
+	cache.MaybeUpsert(ctx, fooUser, laterExpiration, expMetric, ttlMetric)
+	// update.
+	cache.MaybeUpsert(ctx, fooUser, closerExpiration, expMetric, ttlMetric)
+
+	metricFloat := *(findChildMetric(expMetric, fooUser).Gauge.Value)
+	expiration, found := cache.GetExpiration(fooUser)
+
+	// verify that both the cache and metric are in sync.
+	require.Equal(t, true, found)
+	require.Equal(t, closerExpiration, expiration)
+	require.Equal(t, closerExpiration, int64(metricFloat))
+}
+
 func TestPurgePastEntries(t *testing.T) {
 	defer leaktest.AfterTest(t)()