Skip to content

Commit e69eb38

Browse files
authored
access control for forgotten fields (#2052)
1 parent 61be097 commit e69eb38

File tree

12 files changed

+71
-60
lines changed

12 files changed

+71
-60
lines changed

internal/api/access.go

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -150,35 +150,46 @@ func (ai *accessInfo) canChangeMetricByName(create bool, old format.MetricMetaVa
150150
(ai.bitEditDefault && !ai.protectedMetric(oldName) && !ai.protectedMetric(newName))
151151
}
152152

153-
func (ai *accessInfo) CanEditMetric(create bool, old format.MetricMetaValue, new_ format.MetricMetaValue) bool {
154-
if ai.canChangeMetricByName(create, old, new_) {
155-
if ai.bitAdmin {
156-
return true
157-
}
158-
if old.Weight != new_.Weight && !(old.Weight == 0 && new_.Weight == 1) {
159-
return false
160-
}
161-
if preKey(old) != preKey(new_) {
162-
return false
153+
func (ai *accessInfo) CanEditMetric(create bool, old format.MetricMetaValue, new_ format.MetricMetaValue) error {
154+
if !ai.canChangeMetricByName(create, old, new_) {
155+
return fmt.Errorf("access forbidden")
156+
}
157+
if ai.bitAdmin {
158+
return nil
159+
}
160+
if old.Weight != new_.Weight && !(old.Weight == 0 && new_.Weight == 1) {
161+
return fmt.Errorf("access control prevents changing weight")
162+
}
163+
if old.PreKeyFrom != new_.PreKeyFrom {
164+
return fmt.Errorf("access control prevents changing 'presort tag'")
165+
}
166+
if old.PreKeyOnly != new_.PreKeyOnly {
167+
return fmt.Errorf("access control prevents changing 'presort tag only'")
168+
}
169+
if skips(old) != skips(new_) {
170+
return fmt.Errorf("access control prevents changing 'max host', 'min host' or 'sum square'")
171+
}
172+
if old.ShardStrategy != new_.ShardStrategy {
173+
return fmt.Errorf("access control prevents changing sharding strategy")
174+
}
175+
if old.ShardNum != new_.ShardNum {
176+
return fmt.Errorf("access control prevents changing sharding strategy shard")
177+
}
178+
for i := 0; i < max(len(old.Tags), len(new_.Tags)); i++ {
179+
// reducing # of tags implicitly clears RawKind
180+
newRaw := false
181+
if i < len(new_.Tags) {
182+
newRaw = new_.Tags[i].RawKind != ""
163183
}
164-
if preKeyOnly(old) != preKeyOnly(new_) {
165-
return false
184+
oldRaw := false
185+
if i < len(old.Tags) {
186+
oldRaw = old.Tags[i].RawKind != ""
166187
}
167-
if skips(old) != skips(new_) {
168-
return false
188+
if newRaw != oldRaw {
189+
return fmt.Errorf("access control prevents modifying Raw Tag attribute")
169190
}
170-
171-
return true
172191
}
173-
return false
174-
}
175-
176-
func preKey(m format.MetricMetaValue) uint32 {
177-
return m.PreKeyFrom
178-
}
179-
180-
func preKeyOnly(m format.MetricMetaValue) bool {
181-
return m.PreKeyOnly
192+
return nil
182193
}
183194

184195
func skips(m format.MetricMetaValue) [3]bool {

internal/api/access_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,12 +121,12 @@ func TestAccessInfo(t *testing.T) {
121121
ai := accessInfo{
122122
bitEditPrefix: map[string]bool{"foo_": true},
123123
}
124-
require.False(t, ai.CanEditMetric(false, format.MetricMetaValue{Name: "abc"}, format.MetricMetaValue{Name: "foo_bar"}))
125-
require.False(t, ai.CanEditMetric(false, format.MetricMetaValue{Name: "foo_bar"}, format.MetricMetaValue{Name: "abc"}))
126-
require.True(t, ai.CanEditMetric(false, format.MetricMetaValue{Name: "foo_buzz"}, format.MetricMetaValue{Name: "foo_bar"}))
124+
require.False(t, ai.CanEditMetric(false, format.MetricMetaValue{Name: "abc"}, format.MetricMetaValue{Name: "foo_bar"}) == nil)
125+
require.False(t, ai.CanEditMetric(false, format.MetricMetaValue{Name: "foo_bar"}, format.MetricMetaValue{Name: "abc"}) == nil)
126+
require.True(t, ai.CanEditMetric(false, format.MetricMetaValue{Name: "foo_buzz"}, format.MetricMetaValue{Name: "foo_bar"}) == nil)
127127
ai.bitAdmin = true
128-
require.True(t, ai.CanEditMetric(false, format.MetricMetaValue{Name: "abc"}, format.MetricMetaValue{Name: "foo_bar"}))
129-
require.True(t, ai.CanEditMetric(false, format.MetricMetaValue{Name: "foo_bar"}, format.MetricMetaValue{Name: "abc"}))
128+
require.True(t, ai.CanEditMetric(false, format.MetricMetaValue{Name: "abc"}, format.MetricMetaValue{Name: "foo_bar"}) == nil)
129+
require.True(t, ai.CanEditMetric(false, format.MetricMetaValue{Name: "foo_bar"}, format.MetricMetaValue{Name: "abc"}) == nil)
130130
})
131131
})
132132
}
@@ -141,5 +141,5 @@ func canViewMetricNamespaced(ai *accessInfo, metric, namespace string) bool {
141141
}
142142

143143
func canBasicEdit(ai *accessInfo, metric string, create bool) bool {
144-
return ai.CanEditMetric(create, format.MetricMetaValue{Name: metric}, format.MetricMetaValue{Name: metric})
144+
return ai.CanEditMetric(create, format.MetricMetaValue{Name: metric}, format.MetricMetaValue{Name: metric}) == nil
145145
}

internal/api/handler.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1811,8 +1811,8 @@ func (h *Handler) handlePostMetric(ctx context.Context, ai accessInfo, _ string,
18111811
}
18121812
}
18131813
if create {
1814-
if !ai.CanEditMetric(true, metric, metric) {
1815-
return format.MetricMetaValue{}, httpErr(http.StatusForbidden, fmt.Errorf("can't create metric %q", metric.Name))
1814+
if err := ai.CanEditMetric(true, metric, metric); err != nil {
1815+
return format.MetricMetaValue{}, httpErr(http.StatusForbidden, fmt.Errorf("can't create metric %q: %v", metric.Name, err))
18161816
}
18171817
if err = h.applyShardsOnCreate(&metric); err != nil {
18181818
return format.MetricMetaValue{}, httpErr(http.StatusBadRequest, err)
@@ -1835,9 +1835,9 @@ func (h *Handler) handlePostMetric(ctx context.Context, ai accessInfo, _ string,
18351835
return format.MetricMetaValue{},
18361836
httpErr(http.StatusNotFound, fmt.Errorf("metric %q not found (id %d)", metric.Name, metric.MetricID))
18371837
}
1838-
if !ai.CanEditMetric(false, *old, metric) {
1838+
if err := ai.CanEditMetric(false, *old, metric); err != nil {
18391839
return format.MetricMetaValue{},
1840-
httpErr(http.StatusForbidden, fmt.Errorf("can't edit metric %q", old.Name))
1840+
httpErr(http.StatusForbidden, fmt.Errorf("can't edit metric %q: %v", old.Name, err))
18411841
}
18421842
resp, err = h.metadataLoader.SaveMetric(ctx, metric, ai.toMetadata())
18431843
if err != nil {

statshouse-ui/src/admin/pages/FormPage.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ export function EditForm(props: { isReadonly: boolean; adminMode: boolean; isHis
471471
</div>
472472
<div className="row mb-3">
473473
<label htmlFor="resolution" className="col-sm-2 col-form-label">
474-
Presort key
474+
Presort tag
475475
</label>
476476
<div className="col-sm-auto d-flex align-items-center">
477477
<select
@@ -496,7 +496,7 @@ export function EditForm(props: { isReadonly: boolean; adminMode: boolean; isHis
496496
</div>
497497
<div className="row align-items-baseline mb-3">
498498
<label htmlFor="pre_key_only" className="col-sm-2 col-form-label">
499-
Presort key only
499+
Presort tag only
500500
</label>
501501
<div className="col-sm-auto pt-1">
502502
<div className="form-check form-switch">
@@ -584,7 +584,7 @@ export function EditForm(props: { isReadonly: boolean; adminMode: boolean; isHis
584584
</div>
585585
<div className="row align-items-baseline mb-3">
586586
<label htmlFor="fair_key_tag_ids" className="col-sm-2 col-form-label">
587-
Fair key tags
587+
Fair tags
588588
</label>
589589
<div className="col-sm-auto pt-1">
590590
{

website/docs/admin/manage-budgets.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,6 @@ This option is for administrators only.
104104

105105
## Enabling tag-level budgeting
106106

107-
[Enable the "Fair key tags" feature](../guides/edit-metrics.md#fair-key-tags) for a tag to allocate metric resources
107+
[Enable the "Fair tags" feature](../guides/edit-metrics.md#fair-key-tags) for a tag to allocate metric resources
108108
fairly accordingly to the tag values, e.g., different services writing data to the same metric.
109109
Read more about [tag-level budgeting](../overview/concepts.md#tag-level-budgeting-fair-key-tags).

website/docs/guides/edit-metrics.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ Learn [how to edit a metric](#how-to-edit-a-metric) and what the editing options
4141
* [Map the draft tag names to the tag IDs](#map-the-draft-tag-names-to-the-tag-ids)
4242
* [Disabling a metric](#disabling-a-metric)
4343
* [Admin settings](#admin-settings)
44-
* [Fair key tags](#fair-key-tags)
44+
* [Fair tags](#fair-key-tags)
4545
<!-- TOC -->
4646

4747
## How to edit a metric
@@ -254,13 +254,13 @@ These settings are for administrators only:
254254

255255
* _Weight_
256256
* _Mapping Flood Counter_
257-
* _Presort key_
258-
* _Presort key only_
257+
* _Presort tag_
258+
* _Presort tag only_
259259
* _Enable max host_
260260
* _Enable min host_
261261
* _Enable sum square_
262262

263-
### Fair key tags
263+
### Fair tags
264264

265265
Choose the tag to [enable the tag-level budgeting](../overview/concepts.md#tag-level-budgeting-fair-key-tags) for it:
266266

website/docs/overview/concepts.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ To understand StatsHouse deeply, learn the basic metric-related concepts:
3838
* [Fair resource sharing](#fair-resource-sharing)
3939
* [Sampling "mainstays"](#sampling-mainstays)
4040
* [User-guided sampling](#user-guided-sampling)
41-
* [Tag-level budgeting ("Fair key tags")](#tag-level-budgeting-fair-key-tags)
41+
* [Tag-level budgeting ("Fair tags")](#tag-level-budgeting-fair-key-tags)
4242
* [Tag-level budgeting: disabled](#tag-level-budgeting-disabled)
4343
* [Tag-level budgeting: enabled](#tag-level-budgeting-enabled)
4444
<!-- TOC -->
@@ -412,10 +412,10 @@ In this case, you can explicitly specify `counter` for the `value` metric:
412412
This means that the number of events is 6, and the values are sampled—as if the original `value`
413413
was `[1, 1, 2, 2, 3, 3]`
414414

415-
### Tag-level budgeting ("Fair key tags")
415+
### Tag-level budgeting ("Fair tags")
416416

417-
Tag-level budgeting ("Fair key tags") is for communal metrics that receive data from many services.
418-
The ["Fair key tags" feature](../guides/edit-metrics.md#fair-key-tags) allows you to allocate metric resources
417+
Tag-level budgeting ("Fair tags") is for communal metrics that receive data from many services.
418+
The ["Fair tags" feature](../guides/edit-metrics.md#fair-key-tags) allows you to allocate metric resources
419419
fairly — accordingly to the tag values, e.g., services.
420420

421421
#### Tag-level budgeting: disabled
@@ -448,7 +448,7 @@ Let's have a sampling factor (SF) = 10. It means that only one row out of 10 rem
448448
You can assign the `service_id` tag to be a "fair key" — to fairly share the metric budget between two services.
449449
:::
450450

451-
See how to [enable the "Fair key tags" feature](../guides/edit-metrics.md#fair-key-tags) for a tag. The metric budget
451+
See how to [enable the "Fair tags" feature](../guides/edit-metrics.md#fair-key-tags) for a tag. The metric budget
452452
will be fairly shared between the services:
453453
* the rare events from the Service B improve their chances to appear on a StatsHouse graph;
454454
* the intensively-generating Service A gets the reduced budget.

website/docs/tldr.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ Categorize hosts by a _datacenter_, a _cluster_, etc. — not by their names.
111111
specific metrics (as well as groups and namespaces). Use this option sparingly.
112112

113113
:::tip
114-
You can [enable the "Fair key tags" feature](./guides/edit-metrics.md#fair-key-tags) to share the budget fairly
114+
You can [enable the "Fair tags" feature](./guides/edit-metrics.md#fair-key-tags) to share the budget fairly
115115
between the services sending data to the same metric.
116116
Read more about [tag-level budgeting](./overview/concepts.md#tag-level-budgeting-fair-key-tags).
117117
:::

website/i18n/ru/docusaurus-plugin-content-docs/current/admin/manage-budgets.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ StatsHouse обеспечивает справедливое распредел
108108

109109
## Настройка бюджетирования на уровне тегов
110110

111-
[Включите опцию "Fair key tags"](../guides/edit-metrics.md#fair-key-tags),
111+
[Включите опцию "Fair tags"](../guides/edit-metrics.md#fair-key-tags),
112112
чтобы поровну поделить бюджет метрики в соответствии со значениями тега, например между сервисами, которые отправляют
113113
данные в одну и ту же метрику.
114114
Узнайте больше о [бюджетировании на уровне тегов](../overview/concepts.md#бюджетирование-на-уровне-тегов-fair-key-tags).

website/i18n/ru/docusaurus-plugin-content-docs/current/guides/edit-metrics.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import FairKey from '../img/fair-key.png'
4343
* [Драфт-теги](#драфт-теги)
4444
* [Отключение метрики](#отключение-метрики)
4545
* [Настройки для администраторов](#настройки-для-администраторов)
46-
* [Fair key tags](#fair-key-tags)
46+
* [Fair tags](#fair-key-tags)
4747
<!-- TOC -->
4848

4949
## Какие параметры метрики можно редактировать
@@ -258,13 +258,13 @@ StatsHouse извлекает "неизвестные" названия тего
258258

259259
* _Weight_
260260
* _Mapping Flood Counter_
261-
* _Presort key_
262-
* _Presort key only_
261+
* _Presort tag_
262+
* _Presort tag only_
263263
* _Enable max host_
264264
* _Enable min host_
265265
* _Enable sum square_
266266

267-
### Fair key tags
267+
### Fair tags
268268

269269
Чтобы [включить бюджетирование на уровне тега](../overview/concepts.md#бюджетирование-на-уровне-тегов-fair-key-tags),
270270
выберите нужный тег из списка:

0 commit comments

Comments
 (0)