Skip to content

Commit

Permalink
Merge 22f20a8 into debc8fb
Browse files Browse the repository at this point in the history
  • Loading branch information
gyuho authored Jun 17, 2024
2 parents debc8fb + 22f20a8 commit 0734121
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 9 deletions.
27 changes: 19 additions & 8 deletions server/embed/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,24 @@ func (e *Etcd) pickGRPCGatewayServeContext(splitHTTP bool) *serveCtx {
panic("Expect at least one context able to serve grpc")
}

var ErrMissingClientTLSInfoForMetricsURL = errors.New("client TLS key/cert (--cert-file, --key-file) must be provided for metrics url")

func (e *Etcd) createMetricsListener(murl url.URL) (net.Listener, error) {
tlsInfo := &e.cfg.ClientTLSInfo
switch murl.Scheme {
case "http":
tlsInfo = nil
case "https", "unixs":
if e.cfg.ClientTLSInfo.Empty() {
return nil, ErrMissingClientTLSInfoForMetricsURL
}
}
return transport.NewListenerWithOpts(murl.Host, murl.Scheme,
transport.WithTLSInfo(tlsInfo),
transport.WithSocketOpts(&e.cfg.SocketOpts),
)
}

func (e *Etcd) serveMetrics() (err error) {
if e.cfg.Metrics == "extensive" {
grpc_prometheus.EnableHandlingTimeHistogram()
Expand All @@ -833,14 +851,7 @@ func (e *Etcd) serveMetrics() (err error) {
etcdhttp.HandleHealth(e.cfg.logger, metricsMux, e.Server)

for _, murl := range e.cfg.ListenMetricsUrls {
tlsInfo := &e.cfg.ClientTLSInfo
if murl.Scheme == "http" {
tlsInfo = nil
}
ml, err := transport.NewListenerWithOpts(murl.Host, murl.Scheme,
transport.WithTLSInfo(tlsInfo),
transport.WithSocketOpts(&e.cfg.SocketOpts),
)
ml, err := e.createMetricsListener(murl)
if err != nil {
return err
}
Expand Down
24 changes: 24 additions & 0 deletions server/embed/etcd_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package embed

import (
"net/url"
"testing"

"go.etcd.io/etcd/client/pkg/v3/transport"
)

func TestEmptyClientTLSInfo_createMetricsListener(t *testing.T) {
e := &Etcd{
cfg: Config{
ClientTLSInfo: transport.TLSInfo{},
},
}

murl := url.URL{
Scheme: "https",
Host: "localhost:8080",
}
if _, err := e.createMetricsListener(murl); err != ErrMissingClientTLSInfoForMetricsURL {
t.Fatalf("expected error %v, got %v", ErrMissingClientTLSInfoForMetricsURL, err)
}
}
2 changes: 1 addition & 1 deletion server/etcdmain/help.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ Profiling and Monitoring:
--metrics 'basic'
Set level of detail for exported metrics, specify 'extensive' to include server side grpc histogram metrics.
--listen-metrics-urls ''
List of URLs to listen on for the metrics and health endpoints.
List of URLs to listen on for the /metrics and /health endpoints. For https, the client URL TLS info is used.
Logging:
--logger 'zap'
Expand Down
55 changes: 55 additions & 0 deletions tests/e2e/etcd_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"golang.org/x/sync/errgroup"

"go.etcd.io/etcd/pkg/v3/expect"
"go.etcd.io/etcd/server/v3/embed"
"go.etcd.io/etcd/tests/v3/framework/config"
"go.etcd.io/etcd/tests/v3/framework/e2e"
)
Expand Down Expand Up @@ -120,6 +121,60 @@ func TestEtcdUnixPeers(t *testing.T) {
}
}

// TestEtcdListenMetricsURLsWithMissingClientTLSInfo checks that the HTTPs listen metrics URL
// but without the client TLS info will fail its verification.
func TestEtcdListenMetricsURLsWithMissingClientTLSInfo(t *testing.T) {
e2e.SkipInShortMode(t)

tempDir := t.TempDir()
defer os.RemoveAll(tempDir)

caFile, certFiles, keyFiles, err := generateCertsForIPs(tempDir, []net.IP{net.ParseIP("127.0.0.1")})
require.NoError(t, err)

// non HTTP but metrics URL is HTTPS, invalid when the client TLS info is not provided
clientURL := fmt.Sprintf("http://localhost:%d", e2e.EtcdProcessBasePort)
peerURL := fmt.Sprintf("https://localhost:%d", e2e.EtcdProcessBasePort+1)
listenMetricsURL := fmt.Sprintf("https://localhost:%d", e2e.EtcdProcessBasePort+2)

commonArgs := []string{
e2e.BinPath.Etcd,
"--name", "e0",
"--data-dir", tempDir,

"--listen-client-urls", clientURL,
"--advertise-client-urls", clientURL,

"--initial-advertise-peer-urls", peerURL,
"--listen-peer-urls", peerURL,

"--initial-cluster", "e0=" + peerURL,

"--listen-metrics-urls", listenMetricsURL,

"--peer-cert-file", certFiles[0],
"--peer-key-file", keyFiles[0],
"--peer-trusted-ca-file", caFile,
"--peer-client-cert-auth",
}

proc, err := e2e.SpawnCmd(commonArgs, nil)
if err != nil {
t.Fatal(err)
}
defer func() {
if err := proc.Stop(); err != nil {
t.Error(err)
}
proc.Wait() // ensure the port has been released
_ = proc.Close()
}()

if err := e2e.WaitReadyExpectProc(context.TODO(), proc, []string{embed.ErrMissingClientTLSInfoForMetricsURL.Error()}); err != nil {
t.Fatal(err)
}
}

// TestEtcdPeerCNAuth checks that the inter peer auth based on CN of cert is working correctly.
func TestEtcdPeerCNAuth(t *testing.T) {
e2e.SkipInShortMode(t)
Expand Down

0 comments on commit 0734121

Please sign in to comment.