fix(setting): Replacing dynamic client to reduce memory footprint (#115125)

This commit is contained in:
Andres Torres
2025-12-11 10:24:01 -05:00
committed by GitHub
parent 73bcfbcc74
commit 5d7b9c5050
2 changed files with 403 additions and 430 deletions
+170 -82
View File
@@ -2,27 +2,26 @@ package setting
import (
"context"
"encoding/json"
"fmt"
"io"
"net/http"
"time"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/prometheus/client_golang/prometheus"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/trace"
"gopkg.in/ini.v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
utilnet "k8s.io/apimachinery/pkg/util/net"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/client-go/dynamic"
clientrest "k8s.io/client-go/rest"
"k8s.io/client-go/rest"
"k8s.io/client-go/transport"
authlib "github.com/grafana/authlib/authn"
logging "github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/semconv"
)
@@ -38,18 +37,11 @@ const (
ApiGroup = "setting.grafana.app"
apiVersion = "v0alpha1"
resource = "settings"
kind = "Setting"
listKind = "SettingList"
)
var settingGroupVersion = schema.GroupVersionResource{
Group: ApiGroup,
Version: apiVersion,
Resource: resource,
}
var settingGroupListKind = map[schema.GroupVersionResource]string{
settingGroupVersion: listKind,
var settingGroupVersion = schema.GroupVersion{
Group: ApiGroup,
Version: apiVersion,
}
type remoteSettingServiceMetrics struct {
@@ -106,10 +98,10 @@ type Service interface {
}
type remoteSettingService struct {
dynamicClient dynamic.Interface
log logging.Logger
pageSize int64
metrics remoteSettingServiceMetrics
restClient *rest.RESTClient
log logging.Logger
pageSize int64
metrics remoteSettingServiceMetrics
}
var _ Service = (*remoteSettingService)(nil)
@@ -126,7 +118,7 @@ type Config struct {
// At least one of WrapTransport or TokenExchangeClient is required.
WrapTransport transport.WrapperFunc
// TLSClientConfig configures TLS for the client connection.
TLSClientConfig clientrest.TLSClientConfig
TLSClientConfig rest.TLSClientConfig
// QPS limits requests per second (defaults to DefaultQPS).
QPS float32
// Burst allows request bursts above QPS (defaults to DefaultBurst).
@@ -145,29 +137,39 @@ type Setting struct {
Value string `json:"value"`
}
// settingResource represents a single Setting resource from the K8s API.
type settingResource struct {
Spec Setting `json:"spec"`
}
// settingListMetadata contains pagination info from the K8s list response.
type settingListMetadata struct {
Continue string `json:"continue,omitempty"`
}
// New creates a Service from the provided configuration.
func New(config Config) (Service, error) {
log := logging.New(LogPrefix)
dynamicClient, err := getDynamicClient(config, log)
restClient, err := getRestClient(config, log)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to create REST client: %w", err)
}
pageSize := DefaultPageSize
if config.PageSize > 0 {
pageSize = config.PageSize
}
metrics := initMetrics()
return &remoteSettingService{
dynamicClient: dynamicClient,
pageSize: pageSize,
log: log,
metrics: metrics,
restClient: restClient,
log: log,
pageSize: pageSize,
metrics: initMetrics(),
}, nil
}
func (m *remoteSettingService) ListAsIni(ctx context.Context, labelSelector metav1.LabelSelector) (*ini.File, error) {
func (s *remoteSettingService) ListAsIni(ctx context.Context, labelSelector metav1.LabelSelector) (*ini.File, error) {
namespace, ok := request.NamespaceFrom(ctx)
ns := semconv.GrafanaNamespaceName(namespace)
ctx, span := tracer.Start(ctx, "remoteSettingService.ListAsIni",
@@ -178,33 +180,34 @@ func (m *remoteSettingService) ListAsIni(ctx context.Context, labelSelector meta
return nil, tracing.Errorf(span, "missing namespace in context")
}
settings, err := m.List(ctx, labelSelector)
settings, err := s.List(ctx, labelSelector)
if err != nil {
return nil, err
}
iniFile, err := m.toIni(settings)
iniFile, err := toIni(settings)
if err != nil {
return nil, tracing.Error(span, err)
}
return iniFile, nil
}
func (m *remoteSettingService) List(ctx context.Context, labelSelector metav1.LabelSelector) ([]*Setting, error) {
func (s *remoteSettingService) List(ctx context.Context, labelSelector metav1.LabelSelector) ([]*Setting, error) {
namespace, ok := request.NamespaceFrom(ctx)
ns := semconv.GrafanaNamespaceName(namespace)
ctx, span := tracer.Start(ctx, "remoteSettingService.List",
trace.WithAttributes(ns))
defer span.End()
if !ok || namespace == "" {
return nil, tracing.Errorf(span, "missing namespace in context")
}
log := m.log.FromContext(ctx).New(ns.Key, ns.Value, "function", "remoteSettingService.List", "traceId", span.SpanContext().TraceID())
log := s.log.FromContext(ctx).New(ns.Key, ns.Value, "function", "remoteSettingService.List", "traceId", span.SpanContext().TraceID())
startTime := time.Now()
var status string
defer func() {
duration := time.Since(startTime).Seconds()
m.metrics.listDuration.WithLabelValues(status).Observe(duration)
s.metrics.listDuration.WithLabelValues(status).Observe(duration)
}()
selector, err := metav1.LabelSelectorAsSelector(&labelSelector)
@@ -216,64 +219,142 @@ func (m *remoteSettingService) List(ctx context.Context, labelSelector metav1.La
log.Debug("empty selector. Fetching all settings")
}
var allSettings []*Setting
// Pre-allocate with estimated capacity
allSettings := make([]*Setting, 0, s.pageSize*8)
var continueToken string
hasNext := true
totalPages := 0
// Using an upper limit to prevent infinite loops
for hasNext && totalPages < 1000 {
totalPages++
opts := metav1.ListOptions{
Limit: m.pageSize,
Continue: continueToken,
}
if !selector.Empty() {
opts.LabelSelector = selector.String()
}
settingsList, lErr := m.dynamicClient.Resource(settingGroupVersion).Namespace(namespace).List(ctx, opts)
settings, nextToken, lErr := s.fetchPage(ctx, namespace, selector.String(), continueToken)
if lErr != nil {
status = "error"
return nil, tracing.Error(span, lErr)
}
for i := range settingsList.Items {
setting, pErr := parseSettingResource(&settingsList.Items[i])
if pErr != nil {
status = "error"
return nil, tracing.Error(span, pErr)
}
allSettings = append(allSettings, setting)
}
continueToken = settingsList.GetContinue()
allSettings = append(allSettings, settings...)
continueToken = nextToken
if continueToken == "" {
hasNext = false
}
}
status = "success"
m.metrics.listResultSize.WithLabelValues(status).Observe(float64(len(allSettings)))
s.metrics.listResultSize.WithLabelValues(status).Observe(float64(len(allSettings)))
return allSettings, nil
}
func parseSettingResource(setting *unstructured.Unstructured) (*Setting, error) {
spec, found, err := unstructured.NestedMap(setting.Object, "spec")
func (s *remoteSettingService) fetchPage(ctx context.Context, namespace, labelSelector, continueToken string) ([]*Setting, string, error) {
req := s.restClient.Get().
Resource(resource).
Namespace(namespace).
Param("limit", fmt.Sprintf("%d", s.pageSize))
if labelSelector != "" {
req = req.Param("labelSelector", labelSelector)
}
if continueToken != "" {
req = req.Param("continue", continueToken)
}
stream, err := req.Stream(ctx)
if err != nil {
return nil, fmt.Errorf("failed to get spec from setting: %w", err)
}
if !found {
return nil, fmt.Errorf("spec not found in setting %s", setting.GetName())
return nil, "", fmt.Errorf("request failed: %w", err)
}
defer func() { _ = stream.Close() }()
var result Setting
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(spec, &result); err != nil {
return nil, fmt.Errorf("failed to convert spec to Setting: %w", err)
}
return &result, nil
return parseSettingList(stream)
}
func (m *remoteSettingService) toIni(settings []*Setting) (*ini.File, error) {
// parseSettingList parses a SettingList JSON response using token-by-token streaming.
func parseSettingList(r io.Reader) ([]*Setting, string, error) {
decoder := json.NewDecoder(r)
// Currently, first page may have a large number of items.
settings := make([]*Setting, 0, 1600)
var continueToken string
// Skip to the start of the object
if _, err := decoder.Token(); err != nil {
return nil, "", fmt.Errorf("expected start of object: %w", err)
}
for decoder.More() {
// Read field name
tok, err := decoder.Token()
if err != nil {
return nil, "", fmt.Errorf("failed to read field name: %w", err)
}
fieldName, ok := tok.(string)
if !ok {
continue
}
switch fieldName {
case "metadata":
var meta settingListMetadata
if err := decoder.Decode(&meta); err != nil {
return nil, "", fmt.Errorf("failed to decode metadata: %w", err)
}
continueToken = meta.Continue
case "items":
// Parse items array token-by-token
itemSettings, err := parseItems(decoder)
if err != nil {
return nil, "", err
}
settings = append(settings, itemSettings...)
default:
// Skip unknown fields
var skip json.RawMessage
if err := decoder.Decode(&skip); err != nil {
return nil, "", fmt.Errorf("failed to skip field %s: %w", fieldName, err)
}
}
}
return settings, continueToken, nil
}
func parseItems(decoder *json.Decoder) ([]*Setting, error) {
// Expect start of array
tok, err := decoder.Token()
if err != nil {
return nil, fmt.Errorf("expected start of items array: %w", err)
}
if tok != json.Delim('[') {
return nil, fmt.Errorf("expected '[', got %v", tok)
}
settings := make([]*Setting, 0, DefaultPageSize)
// Parse each item
for decoder.More() {
var item settingResource
if err := decoder.Decode(&item); err != nil {
return nil, fmt.Errorf("failed to decode setting item: %w", err)
}
settings = append(settings, &Setting{
Section: item.Spec.Section,
Key: item.Spec.Key,
Value: item.Spec.Value,
})
}
// Consume end of array
if _, err := decoder.Token(); err != nil {
return nil, fmt.Errorf("expected end of items array: %w", err)
}
return settings, nil
}
func toIni(settings []*Setting) (*ini.File, error) {
conf := ini.Empty()
for _, setting := range settings {
if !conf.HasSection(setting.Section) {
@@ -287,7 +368,7 @@ func (m *remoteSettingService) toIni(settings []*Setting) (*ini.File, error) {
return conf, nil
}
func getDynamicClient(config Config, log logging.Logger) (dynamic.Interface, error) {
func getRestClient(config Config, log logging.Logger) (*rest.RESTClient, error) {
if config.URL == "" {
return nil, fmt.Errorf("URL cannot be empty")
}
@@ -296,7 +377,7 @@ func getDynamicClient(config Config, log logging.Logger) (dynamic.Interface, err
}
wrapTransport := config.WrapTransport
if config.WrapTransport == nil {
if wrapTransport == nil {
log.Debug("using default wrapTransport with TokenExchangeClient")
wrapTransport = func(rt http.RoundTripper) http.RoundTripper {
return &authRoundTripper{
@@ -316,13 +397,21 @@ func getDynamicClient(config Config, log logging.Logger) (dynamic.Interface, err
burst = config.Burst
}
return dynamic.NewForConfig(&clientrest.Config{
restConfig := &rest.Config{
Host: config.URL,
WrapTransport: wrapTransport,
TLSClientConfig: config.TLSClientConfig,
WrapTransport: wrapTransport,
QPS: qps,
Burst: burst,
})
// Configure for our API group
APIPath: "/apis",
ContentConfig: rest.ContentConfig{
GroupVersion: &settingGroupVersion,
NegotiatedSerializer: serializer.NewCodecFactory(nil).WithoutConversion(),
},
}
return rest.RESTClientFor(restConfig)
}
// authRoundTripper wraps an HTTP transport with token-based authentication.
@@ -341,10 +430,9 @@ func (a *authRoundTripper) RoundTrip(req *http.Request) (*http.Response, error)
if err != nil {
return nil, fmt.Errorf("failed to exchange token: %w", err)
}
req = utilnet.CloneRequest(req)
req.Header.Set("X-Access-Token", fmt.Sprintf("Bearer %s", token.Token))
return a.transport.RoundTrip(req)
reqCopy := req.Clone(req.Context())
reqCopy.Header.Set("X-Access-Token", fmt.Sprintf("Bearer %s", token.Token))
return a.transport.RoundTrip(reqCopy)
}
func initMetrics() remoteSettingServiceMetrics {
@@ -373,12 +461,12 @@ func initMetrics() remoteSettingServiceMetrics {
return metrics
}
func (m *remoteSettingService) Describe(descs chan<- *prometheus.Desc) {
m.metrics.listDuration.Describe(descs)
m.metrics.listResultSize.Describe(descs)
func (s *remoteSettingService) Describe(descs chan<- *prometheus.Desc) {
s.metrics.listDuration.Describe(descs)
s.metrics.listResultSize.Describe(descs)
}
func (m *remoteSettingService) Collect(metrics chan<- prometheus.Metric) {
m.metrics.listDuration.Collect(metrics)
m.metrics.listResultSize.Collect(metrics)
func (s *remoteSettingService) Collect(metrics chan<- prometheus.Metric) {
s.metrics.listDuration.Collect(metrics)
s.metrics.listResultSize.Collect(metrics)
}
+233 -348
View File
@@ -1,69 +1,36 @@
package setting
import (
"bytes"
"context"
"fmt"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/client-go/dynamic/fake"
k8testing "k8s.io/client-go/testing"
authlib "github.com/grafana/authlib/authn"
"github.com/grafana/grafana/pkg/infra/log"
)
func TestRemoteSettingService_ListAsIni(t *testing.T) {
t.Run("should filter settings by label selector", func(t *testing.T) {
// Create multiple settings, only some matching the selector
setting1 := newUnstructuredSetting("test-namespace", Setting{Section: "database", Key: "type", Value: "postgres"})
setting2 := newUnstructuredSetting("test-namespace", Setting{Section: "server", Key: "port", Value: "3000"})
setting3 := newUnstructuredSetting("test-namespace", Setting{Section: "database", Key: "host", Value: "localhost"})
client := newTestClient(500, setting1, setting2, setting3)
// Create a selector that should match only database settings
selector := metav1.LabelSelector{
MatchLabels: map[string]string{
"section": "database",
},
}
ctx := request.WithNamespace(context.Background(), "test-namespace")
result, err := client.ListAsIni(ctx, selector)
require.NoError(t, err)
assert.NotNil(t, result)
// Should only have database settings, not server settings
assert.True(t, result.HasSection("database"))
assert.Equal(t, "postgres", result.Section("database").Key("type").String())
assert.Equal(t, "localhost", result.Section("database").Key("host").String())
// Should NOT have server settings
assert.False(t, result.HasSection("server"))
})
t.Run("should return all settings with empty selector", func(t *testing.T) {
// Create multiple settings across different sections
setting1 := newUnstructuredSetting("test-namespace", Setting{Section: "server", Key: "port", Value: "3000"})
setting2 := newUnstructuredSetting("test-namespace", Setting{Section: "database", Key: "type", Value: "mysql"})
client := newTestClient(500, setting1, setting2)
// Empty selector should select everything
selector := metav1.LabelSelector{}
settings := []Setting{
{Section: "server", Key: "port", Value: "3000"},
{Section: "database", Key: "type", Value: "mysql"},
}
server := newTestServer(t, settings, "")
defer server.Close()
client := newTestClient(t, server.URL, 500)
ctx := request.WithNamespace(context.Background(), "test-namespace")
result, err := client.ListAsIni(ctx, selector)
result, err := client.ListAsIni(ctx, metav1.LabelSelector{})
require.NoError(t, err)
assert.NotNil(t, result)
// Should have all settings from all sections
assert.True(t, result.HasSection("server"))
assert.Equal(t, "3000", result.Section("server").Key("port").String())
assert.True(t, result.HasSection("database"))
@@ -73,209 +40,168 @@ func TestRemoteSettingService_ListAsIni(t *testing.T) {
func TestRemoteSettingService_List(t *testing.T) {
t.Run("should handle single page response", func(t *testing.T) {
setting := newUnstructuredSetting("test-namespace", Setting{Section: "server", Key: "port", Value: "3000"})
client := newTestClient(500, setting)
settings := []Setting{
{Section: "server", Key: "port", Value: "3000"},
}
server := newTestServer(t, settings, "")
defer server.Close()
client := newTestClient(t, server.URL, 500)
ctx := request.WithNamespace(context.Background(), "test-namespace")
result, err := client.List(ctx, metav1.LabelSelector{})
require.NoError(t, err)
assert.Len(t, result, 1)
spec := result[0]
assert.Equal(t, "server", spec.Section)
assert.Equal(t, "port", spec.Key)
assert.Equal(t, "3000", spec.Value)
assert.Equal(t, "server", result[0].Section)
assert.Equal(t, "port", result[0].Key)
assert.Equal(t, "3000", result[0].Value)
})
t.Run("should handle multiple pages", func(t *testing.T) {
totalPages := 3
pageSize := 5
pages := make([][]*unstructured.Unstructured, totalPages)
for pageNum := 0; pageNum < totalPages; pageNum++ {
for idx := 0; idx < pageSize; idx++ {
item := newUnstructuredSetting(
"test-namespace",
Setting{
Section: fmt.Sprintf("section-%d", pageNum),
Key: fmt.Sprintf("key-%d", idx),
Value: fmt.Sprintf("val-%d-%d", pageNum, idx),
},
)
pages[pageNum] = append(pages[pageNum], item)
}
}
scheme := runtime.NewScheme()
dynamicClient := fake.NewSimpleDynamicClientWithCustomListKinds(scheme, settingGroupListKind)
listCallCount := 0
dynamicClient.PrependReactor("list", "settings", func(action k8testing.Action) (handled bool, ret runtime.Object, err error) {
listCallCount++
continueToken := fmt.Sprintf("continue-%d", listCallCount)
if listCallCount == totalPages {
continueToken = ""
}
if listCallCount <= totalPages {
list := &unstructured.UnstructuredList{
Object: map[string]interface{}{
"apiVersion": ApiGroup + "/" + apiVersion,
"kind": listKind,
},
}
list.SetContinue(continueToken)
for _, item := range pages[listCallCount-1] {
list.Items = append(list.Items, *item)
}
return true, list, nil
}
return false, nil, nil
})
client := &remoteSettingService{
dynamicClient: dynamicClient,
pageSize: int64(pageSize),
log: log.NewNopLogger(),
metrics: initMetrics(),
t.Run("should handle multiple settings", func(t *testing.T) {
settings := []Setting{
{Section: "server", Key: "port", Value: "3000"},
{Section: "database", Key: "host", Value: "localhost"},
{Section: "database", Key: "port", Value: "5432"},
}
server := newTestServer(t, settings, "")
defer server.Close()
client := newTestClient(t, server.URL, 500)
ctx := request.WithNamespace(context.Background(), "test-namespace")
result, err := client.List(ctx, metav1.LabelSelector{})
require.NoError(t, err)
assert.Len(t, result, totalPages*pageSize)
assert.Equal(t, totalPages, listCallCount)
assert.Len(t, result, 3)
})
t.Run("should pass label selector when provided", func(t *testing.T) {
scheme := runtime.NewScheme()
dynamicClient := fake.NewSimpleDynamicClientWithCustomListKinds(scheme, settingGroupListKind)
dynamicClient.PrependReactor("list", "settings", func(action k8testing.Action) (handled bool, ret runtime.Object, err error) {
listAction := action.(k8testing.ListActionImpl)
assert.Equal(t, "app=grafana", listAction.ListOptions.LabelSelector)
return true, &unstructured.UnstructuredList{}, nil
})
client := &remoteSettingService{
dynamicClient: dynamicClient,
pageSize: 500,
log: log.NewNopLogger(),
metrics: initMetrics(),
t.Run("should handle pagination with continue token", func(t *testing.T) {
// First page
page1Settings := []Setting{
{Section: "section-0", Key: "key-0", Value: "value-0"},
{Section: "section-0", Key: "key-1", Value: "value-1"},
}
// Second page
page2Settings := []Setting{
{Section: "section-1", Key: "key-0", Value: "value-2"},
{Section: "section-1", Key: "key-1", Value: "value-3"},
}
requestCount := 0
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
requestCount++
continueToken := r.URL.Query().Get("continue")
var settings []Setting
var nextContinue string
if continueToken == "" {
settings = page1Settings
nextContinue = "page2"
} else {
settings = page2Settings
nextContinue = ""
}
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(generateSettingsJSON(settings, nextContinue)))
}))
defer server.Close()
client := newTestClient(t, server.URL, 2)
ctx := request.WithNamespace(context.Background(), "test-namespace")
_, err := client.List(ctx, metav1.LabelSelector{MatchLabels: map[string]string{"app": "grafana"}})
result, err := client.List(ctx, metav1.LabelSelector{})
require.NoError(t, err)
assert.Len(t, result, 4)
assert.Equal(t, 2, requestCount)
})
t.Run("should stop pagination at 1000 pages", func(t *testing.T) {
scheme := runtime.NewScheme()
dynamicClient := fake.NewSimpleDynamicClientWithCustomListKinds(scheme, settingGroupListKind)
listCallCount := 0
dynamicClient.PrependReactor("list", "settings", func(action k8testing.Action) (handled bool, ret runtime.Object, err error) {
listCallCount++
// Always return a continue token to simulate infinite pagination
list := &unstructured.UnstructuredList{}
list.SetContinue("continue-forever")
return true, list, nil
})
t.Run("should return error when namespace is missing", func(t *testing.T) {
server := newTestServer(t, nil, "")
defer server.Close()
client := &remoteSettingService{
dynamicClient: dynamicClient,
pageSize: 10,
log: log.NewNopLogger(),
metrics: initMetrics(),
}
client := newTestClient(t, server.URL, 500)
ctx := context.Background() // No namespace
ctx := request.WithNamespace(context.Background(), "test-namespace")
_, err := client.List(ctx, metav1.LabelSelector{})
require.NoError(t, err)
assert.Equal(t, 1000, listCallCount, "Should stop at 1000 pages to prevent infinite loops")
})
t.Run("should return error when parsing setting fails", func(t *testing.T) {
scheme := runtime.NewScheme()
dynamicClient := fake.NewSimpleDynamicClientWithCustomListKinds(scheme, settingGroupListKind)
dynamicClient.PrependReactor("list", "settings", func(action k8testing.Action) (handled bool, ret runtime.Object, err error) {
// Return a malformed setting without spec
list := &unstructured.UnstructuredList{
Object: map[string]interface{}{
"apiVersion": ApiGroup + "/" + apiVersion,
"kind": listKind,
},
}
malformedSetting := &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": ApiGroup + "/" + apiVersion,
"kind": kind,
"metadata": map[string]interface{}{
"name": "malformed",
"namespace": "test-namespace",
},
// Missing spec
},
}
list.Items = append(list.Items, *malformedSetting)
return true, list, nil
})
client := &remoteSettingService{
dynamicClient: dynamicClient,
pageSize: 500,
log: log.NewNopLogger(),
metrics: initMetrics(),
}
ctx := request.WithNamespace(context.Background(), "test-namespace")
result, err := client.List(ctx, metav1.LabelSelector{})
require.Error(t, err)
assert.Nil(t, result)
assert.Contains(t, err.Error(), "spec not found")
})
}
func TestParseSettingResource(t *testing.T) {
t.Run("should parse valid setting resource", func(t *testing.T) {
setting := newUnstructuredSetting("test-namespace", Setting{Section: "database", Key: "type", Value: "postgres"})
result, err := parseSettingResource(setting)
require.NoError(t, err)
assert.NotNil(t, result)
assert.Equal(t, "database", result.Section)
assert.Equal(t, "type", result.Key)
assert.Equal(t, "postgres", result.Value)
assert.Contains(t, err.Error(), "missing namespace")
})
t.Run("should return error when spec is missing", func(t *testing.T) {
setting := &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": ApiGroup + "/" + apiVersion,
"kind": kind,
"metadata": map[string]interface{}{
"name": "test-setting",
"namespace": "test-namespace",
},
// No spec
},
}
t.Run("should return error on HTTP error", func(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusInternalServerError)
_, _ = w.Write([]byte("internal server error"))
}))
defer server.Close()
result, err := parseSettingResource(setting)
client := newTestClient(t, server.URL, 500)
ctx := request.WithNamespace(context.Background(), "test-namespace")
result, err := client.List(ctx, metav1.LabelSelector{})
require.Error(t, err)
assert.Nil(t, result)
assert.Contains(t, err.Error(), "spec not found")
})
}
func TestRemoteSettingService_ToIni(t *testing.T) {
func TestParseSettingList(t *testing.T) {
t.Run("should parse valid settings list", func(t *testing.T) {
jsonData := `{
"apiVersion": "setting.grafana.app/v0alpha1",
"kind": "SettingList",
"metadata": {"continue": ""},
"items": [
{"spec": {"section": "database", "key": "type", "value": "postgres"}},
{"spec": {"section": "server", "key": "port", "value": "3000"}}
]
}`
settings, continueToken, err := parseSettingList(strings.NewReader(jsonData))
require.NoError(t, err)
assert.Len(t, settings, 2)
assert.Equal(t, "", continueToken)
assert.Equal(t, "database", settings[0].Section)
assert.Equal(t, "type", settings[0].Key)
assert.Equal(t, "postgres", settings[0].Value)
})
t.Run("should parse continue token", func(t *testing.T) {
jsonData := `{
"apiVersion": "setting.grafana.app/v0alpha1",
"kind": "SettingList",
"metadata": {"continue": "next-page-token"},
"items": []
}`
_, continueToken, err := parseSettingList(strings.NewReader(jsonData))
require.NoError(t, err)
assert.Equal(t, "next-page-token", continueToken)
})
t.Run("should handle empty items", func(t *testing.T) {
jsonData := `{
"apiVersion": "setting.grafana.app/v0alpha1",
"kind": "SettingList",
"metadata": {},
"items": []
}`
settings, _, err := parseSettingList(strings.NewReader(jsonData))
require.NoError(t, err)
assert.Len(t, settings, 0)
})
}
func TestToIni(t *testing.T) {
t.Run("should convert settings to ini format", func(t *testing.T) {
settings := []*Setting{
{Section: "database", Key: "type", Value: "postgres"},
@@ -283,12 +209,7 @@ func TestRemoteSettingService_ToIni(t *testing.T) {
{Section: "server", Key: "http_port", Value: "3000"},
}
client := &remoteSettingService{
pageSize: 500,
log: log.NewNopLogger(),
}
result, err := client.toIni(settings)
result, err := toIni(settings)
require.NoError(t, err)
assert.NotNil(t, result)
@@ -302,12 +223,7 @@ func TestRemoteSettingService_ToIni(t *testing.T) {
t.Run("should handle empty settings list", func(t *testing.T) {
var settings []*Setting
client := &remoteSettingService{
pageSize: 500,
log: log.NewNopLogger(),
}
result, err := client.toIni(settings)
result, err := toIni(settings)
require.NoError(t, err)
assert.NotNil(t, result)
@@ -315,35 +231,13 @@ func TestRemoteSettingService_ToIni(t *testing.T) {
assert.Len(t, sections, 1) // Only default section
})
t.Run("should create section if it does not exist", func(t *testing.T) {
settings := []*Setting{
{Section: "new_section", Key: "new_key", Value: "new_value"},
}
client := &remoteSettingService{
pageSize: 500,
log: log.NewNopLogger(),
}
result, err := client.toIni(settings)
require.NoError(t, err)
assert.True(t, result.HasSection("new_section"))
assert.Equal(t, "new_value", result.Section("new_section").Key("new_key").String())
})
t.Run("should handle multiple keys in same section", func(t *testing.T) {
settings := []*Setting{
{Section: "auth", Key: "disable_login_form", Value: "false"},
{Section: "auth", Key: "disable_signout_menu", Value: "true"},
}
client := &remoteSettingService{
pageSize: 500,
log: log.NewNopLogger(),
}
result, err := client.toIni(settings)
result, err := toIni(settings)
require.NoError(t, err)
assert.True(t, result.HasSection("auth"))
@@ -383,24 +277,23 @@ func TestNew(t *testing.T) {
assert.Equal(t, int64(100), remoteClient.pageSize)
})
t.Run("should use default page size when zero is provided", func(t *testing.T) {
t.Run("should create client with custom QPS and Burst", func(t *testing.T) {
config := Config{
URL: "https://example.com",
WrapTransport: func(rt http.RoundTripper) http.RoundTripper { return rt },
PageSize: 0,
QPS: 50.0,
Burst: 100,
}
client, err := New(config)
require.NoError(t, err)
assert.NotNil(t, client)
remoteClient := client.(*remoteSettingService)
assert.Equal(t, DefaultPageSize, remoteClient.pageSize)
})
t.Run("should return error when config is invalid", func(t *testing.T) {
t.Run("should return error when URL is empty", func(t *testing.T) {
config := Config{
URL: "", // Invalid: empty URL
URL: "",
}
client, err := New(config)
@@ -409,134 +302,126 @@ func TestNew(t *testing.T) {
assert.Nil(t, client)
assert.Contains(t, err.Error(), "URL cannot be empty")
})
}
func TestGetDynamicClient(t *testing.T) {
logger := log.NewNopLogger()
t.Run("should return error when SettingServiceURL is empty", func(t *testing.T) {
config := Config{
URL: "",
WrapTransport: func(rt http.RoundTripper) http.RoundTripper { return rt },
}
client, err := getDynamicClient(config, logger)
require.Error(t, err)
assert.Nil(t, client)
assert.Contains(t, err.Error(), "URL cannot be empty")
})
t.Run("should return error when both TokenExchangeClient and WrapTransport are nil", func(t *testing.T) {
t.Run("should return error when auth is not configured", func(t *testing.T) {
config := Config{
URL: "https://example.com",
TokenExchangeClient: nil,
WrapTransport: nil,
}
client, err := getDynamicClient(config, logger)
client, err := New(config)
require.Error(t, err)
assert.Nil(t, client)
assert.Contains(t, err.Error(), "must set either TokenExchangeClient or WrapTransport")
})
t.Run("should create client with WrapTransport", func(t *testing.T) {
config := Config{
URL: "https://example.com",
WrapTransport: func(rt http.RoundTripper) http.RoundTripper { return rt },
}
client, err := getDynamicClient(config, logger)
require.NoError(t, err)
assert.NotNil(t, client)
})
t.Run("should not fail when QPS and Burst are not provided", func(t *testing.T) {
config := Config{
URL: "https://example.com",
WrapTransport: func(rt http.RoundTripper) http.RoundTripper { return rt },
}
client, err := getDynamicClient(config, logger)
require.NoError(t, err)
assert.NotNil(t, client)
})
t.Run("should not fail when custom QPS and Burst are provided", func(t *testing.T) {
config := Config{
URL: "https://example.com",
WrapTransport: func(rt http.RoundTripper) http.RoundTripper { return rt },
QPS: 10.0,
Burst: 20,
}
client, err := getDynamicClient(config, logger)
require.NoError(t, err)
assert.NotNil(t, client)
})
t.Run("should use WrapTransport when both WrapTransport and TokenExchangeClient are provided", func(t *testing.T) {
t.Run("should use WrapTransport when provided", func(t *testing.T) {
wrapTransportCalled := false
tokenExchangeClient := &authlib.TokenExchangeClient{}
config := Config{
URL: "https://example.com",
TokenExchangeClient: tokenExchangeClient,
URL: "https://example.com",
WrapTransport: func(rt http.RoundTripper) http.RoundTripper {
wrapTransportCalled = true
return rt
},
}
client, err := getDynamicClient(config, logger)
client, err := New(config)
require.NoError(t, err)
assert.NotNil(t, client)
assert.True(t, wrapTransportCalled, "WrapTransport should be called and take precedence over TokenExchangeClient")
assert.True(t, wrapTransportCalled)
})
}
// Helper function to create an unstructured Setting object for tests
func newUnstructuredSetting(namespace string, spec Setting) *unstructured.Unstructured {
// Generate resource name in the format {section}--{key}
name := fmt.Sprintf("%s--%s", spec.Section, spec.Key)
// Helper functions
obj := &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": ApiGroup + "/" + apiVersion,
"kind": kind,
"metadata": map[string]interface{}{
"name": name,
"namespace": namespace,
},
"spec": map[string]interface{}{
"section": spec.Section,
"key": spec.Key,
"value": spec.Value,
},
},
}
// Always set section and key labels
obj.SetLabels(map[string]string{
"section": spec.Section,
"key": spec.Key,
})
return obj
func newTestServer(t *testing.T, settings []Setting, continueToken string) *httptest.Server {
t.Helper()
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(generateSettingsJSON(settings, continueToken)))
}))
}
// Helper function to create a test client with the dynamic fake client
func newTestClient(pageSize int64, objects ...runtime.Object) *remoteSettingService {
scheme := runtime.NewScheme()
dynamicClient := fake.NewSimpleDynamicClientWithCustomListKinds(scheme, settingGroupListKind, objects...)
func newTestClient(t *testing.T, serverURL string, pageSize int64) Service {
t.Helper()
config := Config{
URL: serverURL,
WrapTransport: func(rt http.RoundTripper) http.RoundTripper { return rt },
PageSize: pageSize,
}
client, err := New(config)
require.NoError(t, err)
return client
}
return &remoteSettingService{
dynamicClient: dynamicClient,
pageSize: pageSize,
log: log.NewNopLogger(),
metrics: initMetrics(),
func generateSettingsJSON(settings []Setting, continueToken string) string {
var sb strings.Builder
sb.WriteString(fmt.Sprintf(`{"apiVersion":"setting.grafana.app/v0alpha1","kind":"SettingList","metadata":{"continue":"%s"},"items":[`, continueToken))
for i, s := range settings {
if i > 0 {
sb.WriteString(",")
}
sb.WriteString(fmt.Sprintf(
`{"apiVersion":"setting.grafana.app/v0alpha1","kind":"Setting","metadata":{"name":"%s--%s","namespace":"test-namespace"},"spec":{"section":"%s","key":"%s","value":"%s"}}`,
s.Section, s.Key, s.Section, s.Key, s.Value,
))
}
sb.WriteString(`]}`)
return sb.String()
}
// Benchmark tests for streaming JSON parser
func BenchmarkParseSettingList(b *testing.B) {
jsonData := generateSettingListJSON(4000, 100)
jsonBytes := []byte(jsonData)
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
reader := bytes.NewReader(jsonBytes)
_, _, _ = parseSettingList(reader)
}
}
func BenchmarkParseSettingList_SinglePage(b *testing.B) {
jsonData := generateSettingListJSON(500, 50)
jsonBytes := []byte(jsonData)
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
reader := bytes.NewReader(jsonBytes)
_, _, _ = parseSettingList(reader)
}
}
// generateSettingListJSON generates a K8s-style SettingList JSON response for benchmarks
func generateSettingListJSON(totalSettings, numSections int) string {
var sb strings.Builder
sb.WriteString(`{"apiVersion":"setting.grafana.app/v0alpha1","kind":"SettingList","metadata":{"continue":""},"items":[`)
settingsPerSection := totalSettings / numSections
first := true
for section := 0; section < numSections; section++ {
for key := 0; key < settingsPerSection; key++ {
if !first {
sb.WriteString(",")
}
first = false
sb.WriteString(fmt.Sprintf(
`{"apiVersion":"setting.grafana.app/v0alpha1","kind":"Setting","metadata":{"name":"section-%03d--key-%03d","namespace":"bench-ns"},"spec":{"section":"section-%03d","key":"key-%03d","value":"value-for-section-%d-key-%d"}}`,
section, key, section, key, section, key,
))
}
}
sb.WriteString(`]}`)
return sb.String()
}