From 365c9fef43612546d454b97546fea1d2d25c3e94 Mon Sep 17 00:00:00 2001 From: Aleksei Khamidov <77723673+developer-Bushido@users.noreply.github.com> Date: Tue, 27 Oct 2026 16:04:51 +0200 Subject: [PATCH] cluster: probe Grafana over HTTPS when server.protocol is https The status probe of Grafana always used plain HTTP, so a Grafana with server.protocol set to https or h2 was displayed as Down. The uptime probe used the cluster TLS config instead, although Grafana does not use the cluster certificates, so it failed on clusters with enable_tls. Both probes now take the scheme from server.protocol, resolved the same way as grafana.ini is generated, and skip certificate verification as they only check that Grafana answers. Building the merged config also no longer writes instance settings into the shared server_configs.grafana map. Close #2746 --- pkg/cluster/spec/grafana.go | 38 ++++++++--- pkg/cluster/spec/grafana_test.go | 114 +++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 9 deletions(-) diff --git a/pkg/cluster/spec/grafana.go b/pkg/cluster/spec/grafana.go index 97b037dce7..12e6bcdace 100644 --- a/pkg/cluster/spec/grafana.go +++ b/pkg/cluster/spec/grafana.go @@ -63,6 +63,16 @@ type GrafanaSpec struct { UseVMAsDatasource bool `yaml:"use_vm_as_datasource,omitempty" validate:"use_vm_as_datasource:editable"` } +// mergedConfig returns the settings written to grafana.ini: global +// (server_configs.grafana) overridden by the instance config. global is not +// modified, it is shared by all Grafana instances. +func (s *GrafanaSpec) mergedConfig(global map[string]string) map[string]string { + config := make(map[string]string, len(global)+len(s.Config)) + maps.Copy(config, global) + maps.Copy(config, s.Config) + return config +} + // Role returns the component role of the instance func (s *GrafanaSpec) Role() string { return ComponentGrafana @@ -153,10 +163,10 @@ func (c *GrafanaComponent) Instances() []Instance { s.DeployDir, }, StatusFn: func(_ context.Context, timeout time.Duration, _ *tls.Config, _ ...string) string { - return statusByHost(s.GetManageHost(), s.Port, "/login", timeout, nil) + return statusByHost(s.GetManageHost(), s.Port, "/login", timeout, c.probeTLSConfig(s)) }, - UptimeFn: func(_ context.Context, timeout time.Duration, tlsCfg *tls.Config) time.Duration { - return UptimeByHost(s.GetManageHost(), s.Port, timeout, tlsCfg, "") + UptimeFn: func(_ context.Context, timeout time.Duration, _ *tls.Config) time.Duration { + return UptimeByHost(s.GetManageHost(), s.Port, timeout, c.probeTLSConfig(s), "") }, Component: c, }, @@ -166,6 +176,21 @@ func (c *GrafanaComponent) Instances() []Instance { return ins } +// probeTLSConfig returns the TLS config for the status and uptime probes of s, +// or nil if s serves plain HTTP. +// +// TiUP does not set up TLS for Grafana (see setTLSConfig), so its certificate +// is not issued by the cluster CA, and the probe connects by manage host, which +// the certificate may not name. The probes send no credentials, so the +// certificate is not verified, as with the plain HTTP probe. +func (c *GrafanaComponent) probeTLSConfig(s *GrafanaSpec) *tls.Config { + protocol := s.mergedConfig(c.GetGrafanaConfig())["server.protocol"] + if protocol != "https" && protocol != "h2" { + return nil + } + return &tls.Config{InsecureSkipVerify: true} +} + // GrafanaInstance represent the grafana instance type GrafanaInstance struct { BaseInstance @@ -225,12 +250,7 @@ func (i *GrafanaInstance) InitConfig( return err } - userConfig := i.topo.GetGrafanaConfig() - if userConfig == nil { - userConfig = make(map[string]string) - } - maps.Copy(userConfig, spec.Config) - err := mergeAdditionalGrafanaConf(fp, userConfig) + err := mergeAdditionalGrafanaConf(fp, spec.mergedConfig(i.topo.GetGrafanaConfig())) if err != nil { return err } diff --git a/pkg/cluster/spec/grafana_test.go b/pkg/cluster/spec/grafana_test.go index 1de58af724..f448ddf018 100644 --- a/pkg/cluster/spec/grafana_test.go +++ b/pkg/cluster/spec/grafana_test.go @@ -15,11 +15,17 @@ package spec import ( "context" + "crypto/tls" + "crypto/x509" "fmt" + "net/http" + "net/http/httptest" + "net/url" "os" "os/user" "path" "path/filepath" + "strconv" "strings" "testing" "time" @@ -31,6 +37,7 @@ import ( "github.com/pingcap/tiup/pkg/meta" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gopkg.in/ini.v1" ) func TestLocalDashboards(t *testing.T) { @@ -434,3 +441,110 @@ func TestVictoriaMetricsDefaultDatasource(t *testing.T) { require.NoError(t, err) assert.Contains(t, string(dsContent), "name: test-cluster-vm") } + +// TestGrafanaProbeProtocol checks that the status and uptime probes follow +// server.protocol and do not use the cluster TLS config. +func TestGrafanaProbeProtocol(t *testing.T) { + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/metrics" { + fmt.Fprintf(w, "process_start_time_seconds %d\n", time.Now().Add(-time.Hour).Unix()) + } + }) + httpServer := httptest.NewServer(handler) + t.Cleanup(httpServer.Close) + httpsServer := httptest.NewTLSServer(handler) + t.Cleanup(httpsServer.Close) + + // the cluster TLS config of a cluster with enable_tls, Grafana does not use it + clusterTLS := &tls.Config{RootCAs: x509.NewCertPool()} + + tests := []struct { + name string + serverConfig map[string]string + instanceConfig map[string]string + tlsServer bool + clusterTLS *tls.Config + status string + }{ + {name: "http by default", status: "Up"}, + {name: "http with cluster tls", clusterTLS: clusterTLS, status: "Up"}, + {name: "https in instance config", instanceConfig: map[string]string{"server.protocol": "https"}, tlsServer: true, status: "Up"}, + {name: "h2 in instance config", instanceConfig: map[string]string{"server.protocol": "h2"}, tlsServer: true, status: "Up"}, + {name: "https in server_configs", serverConfig: map[string]string{"server.protocol": "https"}, tlsServer: true, clusterTLS: clusterTLS, status: "Up"}, + {name: "instance config overrides server_configs", serverConfig: map[string]string{"server.protocol": "https"}, instanceConfig: map[string]string{"server.protocol": "http"}, tlsServer: true, status: "Down"}, + {name: "http probe of https grafana", tlsServer: true, status: "Down"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + server := httpServer + if tt.tlsServer { + server = httpsServer + } + u, err := url.Parse(server.URL) + require.NoError(t, err) + port, err := strconv.Atoi(u.Port()) + require.NoError(t, err) + + topo := new(Specification) + topo.ServerConfigs.Grafana = tt.serverConfig + topo.Grafanas = append(topo.Grafanas, &GrafanaSpec{ + Host: u.Hostname(), + Port: port, + Config: tt.instanceConfig, + }) + ins := (&GrafanaComponent{topo}).Instances() + require.Len(t, ins, 1) + + ctx := context.Background() + require.Equal(t, tt.status, ins[0].Status(ctx, 5*time.Second, tt.clusterTLS)) + uptime := ins[0].Uptime(ctx, 5*time.Second, tt.clusterTLS) + if tt.status == "Up" { + require.GreaterOrEqual(t, uptime, time.Hour) + } else { + require.Zero(t, uptime) + } + }) + } +} + +// TestGrafanaMergedConfig checks that the instance config overrides +// server_configs.grafana and the shared map is not modified. +func TestGrafanaMergedConfig(t *testing.T) { + global := map[string]string{"log.level": "info", "server.protocol": "http"} + a := &GrafanaSpec{Config: map[string]string{"server.protocol": "https"}} + b := &GrafanaSpec{} + + require.Equal(t, map[string]string{"log.level": "info", "server.protocol": "https"}, a.mergedConfig(global)) + require.Equal(t, map[string]string{"log.level": "info", "server.protocol": "http"}, b.mergedConfig(global)) + require.Equal(t, map[string]string{"log.level": "info", "server.protocol": "http"}, global) + require.Empty(t, b.mergedConfig(nil)) +} + +// TestGrafanaInitConfigInstanceOverride checks that the instance config of one +// Grafana does not leak into the grafana.ini of another one. +func TestGrafanaInitConfigInstanceOverride(t *testing.T) { + ctx := ctxt.New(context.Background(), 0, logprinter.NewLogger("")) + + topo := new(Specification) + topo.ServerConfigs.Grafana = map[string]string{"server.enable_gzip": "true"} + topo.Monitors = []*PrometheusSpec{{Host: "127.0.0.1", Port: 9090}} + topo.Grafanas = []*GrafanaSpec{ + {Host: "127.0.0.1", Port: 3000, Config: map[string]string{"server.protocol": "https"}}, + {Host: "127.0.0.1", Port: 3001}, + } + + var servers []*ini.Section + for _, ins := range (&GrafanaComponent{topo}).Instances() { + paths := meta.DirPaths{Deploy: t.TempDir(), Cache: t.TempDir()} + require.NoError(t, ins.InitConfig(ctx, &mockExecutor{}, "test-cluster", "v8.5.0", "tidb", paths)) + cfg, err := ini.Load(filepath.Join(paths.Deploy, "conf", "grafana.ini")) + require.NoError(t, err) + servers = append(servers, cfg.Section("server")) + } + + require.Equal(t, "https", servers[0].Key("protocol").String()) + require.Equal(t, "true", servers[0].Key("enable_gzip").String()) + require.False(t, servers[1].HasKey("protocol")) + require.Equal(t, "true", servers[1].Key("enable_gzip").String()) + require.Equal(t, map[string]string{"server.enable_gzip": "true"}, topo.ServerConfigs.Grafana) +}