From 245d13196764a0e7368b1154e244ce784d31c446 Mon Sep 17 00:00:00 2001
From: Damien Grisonnet <dgrisonn@redhat.com>
Date: Thu, 10 Aug 2023 17:01:17 +0200
Subject: [PATCH] apiserver/etcd3: fix segv during metric collection

Fix a segfault when collecting the storage size metrics when the getters
used to collect the data on etcd haven't been initialized properly. This
happens when the EtcdOptions are not applied which is the case for
aggregated apiservers that don't care about storage.

Signed-off-by: Damien Grisonnet <dgrisonn@redhat.com>

Kubernetes-commit: c6efaf16c1ed07ce37485b7a272628f653cbf06f
---
 pkg/storage/etcd3/metrics/metrics.go      |  2 +-
 pkg/storage/etcd3/metrics/metrics_test.go | 58 +++++++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/pkg/storage/etcd3/metrics/metrics.go b/pkg/storage/etcd3/metrics/metrics.go
index 091eef85a..ac023d55d 100644
--- a/pkg/storage/etcd3/metrics/metrics.go
+++ b/pkg/storage/etcd3/metrics/metrics.go
@@ -85,7 +85,7 @@ var (
 		[]string{"endpoint"},
 	)
 	storageSizeDescription   = compbasemetrics.NewDesc("apiserver_storage_size_bytes", "Size of the storage database file physically allocated in bytes.", []string{"cluster"}, nil, compbasemetrics.ALPHA, "")
-	storageMonitor           = &monitorCollector{}
+	storageMonitor           = &monitorCollector{monitorGetter: func() ([]Monitor, error) { return nil, nil }}
 	etcdEventsReceivedCounts = compbasemetrics.NewCounterVec(
 		&compbasemetrics.CounterOpts{
 			Subsystem:      "apiserver",
diff --git a/pkg/storage/etcd3/metrics/metrics_test.go b/pkg/storage/etcd3/metrics/metrics_test.go
index 93d15e4af..6190131f0 100644
--- a/pkg/storage/etcd3/metrics/metrics_test.go
+++ b/pkg/storage/etcd3/metrics/metrics_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package metrics
 
 import (
+	"context"
 	"errors"
 	"strings"
 	"testing"
@@ -179,3 +180,60 @@ etcd_request_errors_total{operation="foo",type="bar"} 1
 		})
 	}
 }
+
+func TestStorageSizeCollector(t *testing.T) {
+	registry := metrics.NewKubeRegistry()
+	registry.CustomMustRegister(storageMonitor)
+
+	testCases := []struct {
+		desc           string
+		getterOverride func() ([]Monitor, error)
+		err            error
+		want           string
+	}{
+		{
+			desc: "fake etcd getter",
+			getterOverride: func() ([]Monitor, error) {
+				return []Monitor{fakeEtcdMonitor{storageSize: 1e9}}, nil
+			},
+			err: nil,
+			want: `# HELP apiserver_storage_size_bytes [ALPHA] Size of the storage database file physically allocated in bytes.
+			# TYPE apiserver_storage_size_bytes gauge
+			apiserver_storage_size_bytes{cluster="etcd-0"} 1e+09
+			`,
+		},
+		{
+			desc:           "getters not configured",
+			getterOverride: nil,
+			err:            nil,
+			want:           ``,
+		},
+	}
+
+	for _, test := range testCases {
+		t.Run(test.desc, func(t *testing.T) {
+			defer registry.Reset()
+			if test.getterOverride != nil {
+				oldGetter := storageMonitor.monitorGetter
+				defer SetStorageMonitorGetter(oldGetter)
+				SetStorageMonitorGetter(test.getterOverride)
+			}
+			if err := testutil.GatherAndCompare(registry, strings.NewReader(test.want), "apiserver_storage_size_bytes"); err != nil {
+				t.Fatal(err)
+			}
+		})
+	}
+
+}
+
+type fakeEtcdMonitor struct {
+	storageSize int64
+}
+
+func (m fakeEtcdMonitor) Monitor(_ context.Context) (StorageMetrics, error) {
+	return StorageMetrics{Size: m.storageSize}, nil
+}
+
+func (m fakeEtcdMonitor) Close() error {
+	return nil
+}