From b90e597468e23692382d40995c04b7cbd3a5c134 Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Fri, 3 Jan 2025 15:15:50 -0600 Subject: [PATCH 1/5] Add option to hide tokens from status page --- ring/basic_lifecycler.go | 4 +- ring/lifecycler.go | 4 +- ring/ring.go | 5 +- ring/ring_http.go | 20 +++-- ring/ring_http_test.go | 154 +++++++++++++++++++++++++++++++++++++++ ring/ring_status.gohtml | 35 +++++---- 6 files changed, 198 insertions(+), 24 deletions(-) create mode 100644 ring/ring_http_test.go diff --git a/ring/basic_lifecycler.go b/ring/basic_lifecycler.go index c0860d682..9de657fc1 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 + // HideTokens allows tokens to be hidden from e.g. the status page, for use in contexts which do not utilize tokens. + HideTokens 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.HideTokens).handle(w, req) } diff --git a/ring/lifecycler.go b/ring/lifecycler.go index 083f112bd..33c2feddf 100644 --- a/ring/lifecycler.go +++ b/ring/lifecycler.go @@ -55,6 +55,8 @@ type LifecyclerConfig struct { // Injected internally ListenPort int `yaml:"-"` + // HideTokens allows tokens to be hidden from e.g. the status page, for use in contexts which do not utilize tokens. + HideTokens 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.HideTokens).handle(w, req) } // unregister removes our entry from consul. diff --git a/ring/ring.go b/ring/ring.go index d47eb8fe2..daa10458c 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:"-"` + // HideTokens allows tokens to be hidden from 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. + HideTokens 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.HideTokens).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..6d743b565 --- /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(ctx context.Context, fn 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 }} From 4a9f7046c18a345e89c64a5f482130e7c14ab9ad Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Fri, 3 Jan 2025 15:17:00 -0600 Subject: [PATCH 2/5] drive-by: use a per-case recorder --- ring/partition_ring_http_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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) From f9078ff6b205a2509032da3f089d4c422c0effa5 Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Fri, 3 Jan 2025 15:58:40 -0600 Subject: [PATCH 3/5] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) 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. From e1ef5f676fd845d26114f2492b8f4eefff7f2d41 Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Fri, 3 Jan 2025 16:00:35 -0600 Subject: [PATCH 4/5] linter --- ring/ring_http_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ring/ring_http_test.go b/ring/ring_http_test.go index 6d743b565..8c27016f7 100644 --- a/ring/ring_http_test.go +++ b/ring/ring_http_test.go @@ -149,6 +149,6 @@ func (f *fakeRingAccess) getRing(context.Context) (*Desc, error) { return f.desc, nil } -func (f *fakeRingAccess) casRing(ctx context.Context, fn func(in interface{}) (out interface{}, retry bool, err error)) error { +func (f *fakeRingAccess) casRing(_ context.Context, _ func(in interface{}) (out interface{}, retry bool, err error)) error { return nil } From 6aeb9d4a482799d0f5059a402dd80f57bb006a1b Mon Sep 17 00:00:00 2001 From: Alex Weaver Date: Mon, 6 Jan 2025 13:34:46 -0600 Subject: [PATCH 5/5] Rename to HideTokensInStatusPage --- ring/basic_lifecycler.go | 6 +++--- ring/lifecycler.go | 6 +++--- ring/ring.go | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/ring/basic_lifecycler.go b/ring/basic_lifecycler.go index 9de657fc1..1a2e10380 100644 --- a/ring/basic_lifecycler.go +++ b/ring/basic_lifecycler.go @@ -53,8 +53,8 @@ type BasicLifecyclerConfig struct { HeartbeatTimeout time.Duration TokensObservePeriod time.Duration NumTokens int - // HideTokens allows tokens to be hidden from e.g. the status page, for use in contexts which do not utilize tokens. - HideTokens bool + // 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. @@ -548,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, l.cfg.HideTokens).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 33c2feddf..bb9f1e8f3 100644 --- a/ring/lifecycler.go +++ b/ring/lifecycler.go @@ -55,8 +55,8 @@ type LifecyclerConfig struct { // Injected internally ListenPort int `yaml:"-"` - // HideTokens allows tokens to be hidden from e.g. the status page, for use in contexts which do not utilize tokens. - HideTokens 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. + 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. @@ -1090,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, i.cfg.HideTokens).handle(w, req) + newRingPageHandler(i, i.cfg.HeartbeatTimeout, i.cfg.HideTokensInStatusPage).handle(w, req) } // unregister removes our entry from consul. diff --git a/ring/ring.go b/ring/ring.go index daa10458c..62a49a6d8 100644 --- a/ring/ring.go +++ b/ring/ring.go @@ -150,9 +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:"-"` - // HideTokens allows tokens to be hidden from e.g. the status page, for use in contexts which do not utilize tokens. + // 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. - HideTokens bool `yaml:"-"` + HideTokensInStatusPage bool `yaml:"-"` } // RegisterFlags adds the flags required to config this to the given FlagSet with a specified prefix @@ -1226,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, r.cfg.HideTokens).handle(w, req) + newRingPageHandler(r, r.cfg.HeartbeatTimeout, r.cfg.HideTokensInStatusPage).handle(w, req) } // InstancesCount returns the number of instances in the ring.