From 3702098cbd0c676b648945ee812313ca409a8daa Mon Sep 17 00:00:00 2001 From: Alexander Weaver Date: Mon, 6 Jan 2025 14:57:46 -0600 Subject: [PATCH] chore: Add option to hide tokens from ring statuspage (#633) * Add option to hide tokens from status page * drive-by: use a per-case recorder * changelog * linter * Rename to HideTokensInStatusPage --- CHANGELOG.md | 1 + ring/basic_lifecycler.go | 4 +- ring/lifecycler.go | 4 +- ring/partition_ring_http_test.go | 5 +- ring/ring.go | 5 +- ring/ring_http.go | 20 ++-- ring/ring_http_test.go | 154 +++++++++++++++++++++++++++++++ ring/ring_status.gohtml | 35 ++++--- 8 files changed, 202 insertions(+), 26 deletions(-) create mode 100644 ring/ring_http_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index ee1781868..cb4029f73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,7 @@ * [FEATURE] Add methods `Increment`, `FlushAll`, `CompareAndSwap`, `Touch` to `cache.MemcachedClient` #477 * [FEATURE] Add `concurrency.ForEachJobMergeResults()` utility function. #486 * [FEATURE] Add `ring.DoMultiUntilQuorumWithoutSuccessfulContextCancellation()`. #495 +* [ENHANCEMENT] Add option to hide token information in ring status page #633 * [ENHANCEMENT] Display token information in partition ring status page #631 * [ENHANCEMENT] Add ability to log all source hosts from http header instead of only the first one. #444 * [ENHANCEMENT] Add configuration to customize backoff for the gRPC clients. diff --git a/ring/basic_lifecycler.go b/ring/basic_lifecycler.go index c0860d682..1a2e10380 100644 --- a/ring/basic_lifecycler.go +++ b/ring/basic_lifecycler.go @@ -53,6 +53,8 @@ type BasicLifecyclerConfig struct { HeartbeatTimeout time.Duration TokensObservePeriod time.Duration NumTokens int + // HideTokensInStatusPage allows tokens to be hidden from management tools e.g. the status page, for use in contexts which do not utilize tokens. + HideTokensInStatusPage bool // If true lifecycler doesn't unregister instance from the ring when it's stopping. Default value is false, // which means unregistering. @@ -546,5 +548,5 @@ func (l *BasicLifecycler) getRing(ctx context.Context) (*Desc, error) { } func (l *BasicLifecycler) ServeHTTP(w http.ResponseWriter, req *http.Request) { - newRingPageHandler(l, l.cfg.HeartbeatTimeout).handle(w, req) + newRingPageHandler(l, l.cfg.HeartbeatTimeout, l.cfg.HideTokensInStatusPage).handle(w, req) } diff --git a/ring/lifecycler.go b/ring/lifecycler.go index 083f112bd..bb9f1e8f3 100644 --- a/ring/lifecycler.go +++ b/ring/lifecycler.go @@ -55,6 +55,8 @@ type LifecyclerConfig struct { // Injected internally ListenPort int `yaml:"-"` + // HideTokensInStatusPage allows tokens to be hidden from management tools e.g. the status page, for use in contexts which do not utilize tokens. + HideTokensInStatusPage bool `yaml:"-"` // If set, specifies the TokenGenerator implementation that will be used for generating tokens. // Default value is nil, which means that RandomTokenGenerator is used. @@ -1088,7 +1090,7 @@ func (i *Lifecycler) getRing(ctx context.Context) (*Desc, error) { } func (i *Lifecycler) ServeHTTP(w http.ResponseWriter, req *http.Request) { - newRingPageHandler(i, i.cfg.HeartbeatTimeout).handle(w, req) + newRingPageHandler(i, i.cfg.HeartbeatTimeout, i.cfg.HideTokensInStatusPage).handle(w, req) } // unregister removes our entry from consul. diff --git a/ring/partition_ring_http_test.go b/ring/partition_ring_http_test.go index aef11603d..13293ff32 100644 --- a/ring/partition_ring_http_test.go +++ b/ring/partition_ring_http_test.go @@ -60,9 +60,8 @@ func TestPartitionRingPageHandler_ViewPage(t *testing.T) { nil, ) - recorder := httptest.NewRecorder() - t.Run("displays expected partition info", func(t *testing.T) { + recorder := httptest.NewRecorder() handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/partition-ring", nil)) assert.Equal(t, http.StatusOK, recorder.Code) @@ -97,6 +96,7 @@ func TestPartitionRingPageHandler_ViewPage(t *testing.T) { }) t.Run("displays Show Tokens button by default", func(t *testing.T) { + recorder := httptest.NewRecorder() handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/partition-ring", nil)) assert.Equal(t, http.StatusOK, recorder.Code) @@ -108,6 +108,7 @@ func TestPartitionRingPageHandler_ViewPage(t *testing.T) { }) t.Run("displays tokens when Show Tokens is enabled", func(t *testing.T) { + recorder := httptest.NewRecorder() handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/partition-ring?tokens=true", nil)) assert.Equal(t, http.StatusOK, recorder.Code) diff --git a/ring/ring.go b/ring/ring.go index d47eb8fe2..62a49a6d8 100644 --- a/ring/ring.go +++ b/ring/ring.go @@ -150,6 +150,9 @@ type Config struct { // Whether the shuffle-sharding subring cache is disabled. This option is set // internally and never exposed to the user. SubringCacheDisabled bool `yaml:"-"` + // HideTokensInStatusPage allows tokens to be hidden from management tools e.g. the status page, for use in contexts which do not utilize tokens. + // This option is set internally and never exposed to the user. + HideTokensInStatusPage bool `yaml:"-"` } // RegisterFlags adds the flags required to config this to the given FlagSet with a specified prefix @@ -1223,7 +1226,7 @@ func (r *Ring) getRing(_ context.Context) (*Desc, error) { } func (r *Ring) ServeHTTP(w http.ResponseWriter, req *http.Request) { - newRingPageHandler(r, r.cfg.HeartbeatTimeout).handle(w, req) + newRingPageHandler(r, r.cfg.HeartbeatTimeout, r.cfg.HideTokensInStatusPage).handle(w, req) } // InstancesCount returns the number of instances in the ring. diff --git a/ring/ring_http.go b/ring/ring_http.go index 67249e2b4..d961d8b15 100644 --- a/ring/ring_http.go +++ b/ring/ring_http.go @@ -30,9 +30,12 @@ var defaultPageTemplate = template.Must(template.New("webpage").Funcs(template.F }).Parse(defaultPageContent)) type httpResponse struct { - Ingesters []ingesterDesc `json:"shards"` - Now time.Time `json:"now"` - ShowTokens bool `json:"-"` + Ingesters []ingesterDesc `json:"shards"` + Now time.Time `json:"now"` + // ShowTokens indicates whether the Show Tokens button is clicked. + ShowTokens bool `json:"-"` + // DisableTokens hides the concept of tokens entirely in the page, across all elements. + DisableTokens bool `json:"-"` } type ingesterDesc struct { @@ -57,12 +60,14 @@ type ringAccess interface { type ringPageHandler struct { r ringAccess heartbeatTimeout time.Duration + disableTokens bool } -func newRingPageHandler(r ringAccess, heartbeatTimeout time.Duration) *ringPageHandler { +func newRingPageHandler(r ringAccess, heartbeatTimeout time.Duration, disableTokens bool) *ringPageHandler { return &ringPageHandler{ r: r, heartbeatTimeout: heartbeatTimeout, + disableTokens: disableTokens, } } @@ -132,9 +137,10 @@ func (h *ringPageHandler) handle(w http.ResponseWriter, req *http.Request) { tokensParam := req.URL.Query().Get("tokens") renderHTTPResponse(w, httpResponse{ - Ingesters: ingesters, - Now: now, - ShowTokens: tokensParam == "true", + Ingesters: ingesters, + Now: now, + ShowTokens: tokensParam == "true", + DisableTokens: h.disableTokens, }, defaultPageTemplate, req) } diff --git a/ring/ring_http_test.go b/ring/ring_http_test.go new file mode 100644 index 000000000..8c27016f7 --- /dev/null +++ b/ring/ring_http_test.go @@ -0,0 +1,154 @@ +package ring + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "regexp" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestRingPageHandler_handle(t *testing.T) { + now := time.Now() + ring := fakeRingAccess{ + desc: &Desc{ + Ingesters: map[string]InstanceDesc{ + "1": { + Zone: "zone-a", + State: ACTIVE, + Addr: "addr-a", + Timestamp: now.Unix(), + Tokens: []uint32{1000000, 3000000, 6000000}, + }, + "2": { + Zone: "zone-b", + State: ACTIVE, + Addr: "addr-b", + Timestamp: now.Unix(), + Tokens: []uint32{2000000, 4000000, 5000000, 7000000}, + }, + }, + }, + } + handler := newRingPageHandler(&ring, 10*time.Second, false) + + t.Run("displays instance info", func(t *testing.T) { + recorder := httptest.NewRecorder() + handler.handle(recorder, httptest.NewRequest(http.MethodGet, "/ring", nil)) + + assert.Equal(t, http.StatusOK, recorder.Code) + assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "1", "", + "", "zone-a", "", + "", "ACTIVE", "", + "", "addr-a", "", + }, `\s*`))), recorder.Body.String()) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "3", "", + "", "100%", "", + }, `\s*`))), recorder.Body.String()) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "2", "", + "", "zone-b", "", + "", "ACTIVE", "", + "", "addr-b", "", + }, `\s*`))), recorder.Body.String()) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "4", "", + "", "100%", "", + }, `\s*`))), recorder.Body.String()) + }) + + t.Run("displays Show Tokens button by default", func(t *testing.T) { + recorder := httptest.NewRecorder() + handler.handle(recorder, httptest.NewRequest(http.MethodGet, "/ring", nil)) + + assert.Equal(t, http.StatusOK, recorder.Code) + assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + ``, + }, `\s*`))), recorder.Body.String()) + }) + + t.Run("displays tokens when Show Tokens is enabled", func(t *testing.T) { + recorder := httptest.NewRecorder() + handler.handle(recorder, httptest.NewRequest(http.MethodGet, "/ring?tokens=true", nil)) + + assert.Equal(t, http.StatusOK, recorder.Code) + assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + ``, + }, `\s*`))), recorder.Body.String()) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "

", "Instance: 1", "

", + "

", "Tokens:
", "1000000", "3000000", "6000000", "

", + }, `\s*`))), recorder.Body.String()) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "

", "Instance: 2", "

", + "

", "Tokens:
", "2000000", "4000000", "5000000", "7000000", "

", + }, `\s*`))), recorder.Body.String()) + }) + + tokenDisabledHandler := newRingPageHandler(&ring, 10*time.Second, true) + + t.Run("hides token columns when tokens are disabled", func(t *testing.T) { + recorder := httptest.NewRecorder() + tokenDisabledHandler.handle(recorder, httptest.NewRequest(http.MethodGet, "/ring", nil)) + + assert.Equal(t, http.StatusOK, recorder.Code) + assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) + + assert.NotRegexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "Tokens", "", + "", "Ownership", "", + }, `\s*`))), recorder.Body.String()) + + assert.NotRegexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "3", "", + "", "100%", "", + }, `\s*`))), recorder.Body.String()) + + assert.NotRegexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "4", "", + "", "100%", "", + }, `\s*`))), recorder.Body.String()) + }) + + t.Run("hides Show Tokens button when tokens are disabled", func(t *testing.T) { + recorder := httptest.NewRecorder() + tokenDisabledHandler.handle(recorder, httptest.NewRequest(http.MethodGet, "/ring", nil)) + + assert.Equal(t, http.StatusOK, recorder.Code) + assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) + + assert.NotRegexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + `input type="button" value="Show Tokens"`, + }, `\s*`))), recorder.Body.String()) + }) +} + +type fakeRingAccess struct { + desc *Desc +} + +func (f *fakeRingAccess) getRing(context.Context) (*Desc, error) { + return f.desc, nil +} + +func (f *fakeRingAccess) casRing(_ context.Context, _ func(in interface{}) (out interface{}, retry bool, err error)) error { + return nil +} diff --git a/ring/ring_status.gohtml b/ring/ring_status.gohtml index 157f8d89e..055873f3b 100644 --- a/ring/ring_status.gohtml +++ b/ring/ring_status.gohtml @@ -21,8 +21,10 @@ Read-Only Read-Only Updated Last Heartbeat + {{ if not .DisableTokens }} Tokens Ownership + {{ end }} Actions @@ -46,8 +48,10 @@ {{ .ReadOnlyUpdatedTimestamp | timeOrEmptyString }} {{ end }} {{ .HeartbeatTimestamp | durationSince }} ago ({{ .HeartbeatTimestamp.Format "15:04:05.999" }}) + {{ if not $.DisableTokens }} {{ .NumTokens }} {{ .Ownership | humanFloat }}% + {{ end }} @@ -56,21 +60,24 @@
- {{ if .ShowTokens }} - - {{ else }} - - {{ end }} - {{ if .ShowTokens }} - {{ range $i, $ing := .Ingesters }} -

Instance: {{ .ID }}

-

- Tokens:
- {{ range $token := .Tokens }} - {{ $token }} - {{ end }} -

+ {{ if not .DisableTokens}} + {{ if .ShowTokens }} + + {{ else }} + + {{ end }} + + {{ if .ShowTokens }} + {{ range $i, $ing := .Ingesters }} +

Instance: {{ .ID }}

+

+ Tokens:
+ {{ range $token := .Tokens }} + {{ $token }} + {{ end }} +

+ {{ end }} {{ end }} {{ end }}