Merge #142682

142682: security:	cache certificate expiration metrics as pointers r=angles-n-daemons a=angles-n-daemons

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.

Co-authored-by: Brian Dillmann <[email protected]>

Author: craig[bot]

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)()