Skip to content

Commit 9629c2c

Browse files
committed
PRW2: add labelRef validation
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
1 parent db252aa commit 9629c2c

File tree

4 files changed

+148
-12
lines changed

4 files changed

+148
-12
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
* [ENHANCEMENT] Ingester: Add `cortex_ingester_tsdb_head_stale_series` metric to keep track of number of stale series on head. #7071
8686
* [ENHANCEMENT] Expose more Go runtime metrics. #7070
8787
* [ENHANCEMENT] Distributor: Filter out label with empty value. #7069
88+
* [ENHANCEMENT] Distributor: Add a validation for label references. #7074
8889
* [BUGFIX] Ingester: Avoid error or early throttling when READONLY ingesters are present in the ring #6517
8990
* [BUGFIX] Ingester: Fix labelset data race condition. #6573
9091
* [BUGFIX] Compactor: Cleaner should not put deletion marker for blocks with no-compact marker. #6576

pkg/cortexpb/compatv2.go

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,34 @@
11
package cortexpb
22

3-
import "github.com/prometheus/prometheus/model/labels"
3+
import (
4+
"fmt"
45

5-
func (e *ExemplarV2) ToLabels(b *labels.ScratchBuilder, symbols []string) labels.Labels {
6+
"github.com/prometheus/prometheus/model/labels"
7+
)
8+
9+
func (e *ExemplarV2) ToLabels(b *labels.ScratchBuilder, symbols []string) (labels.Labels, error) {
610
return desymbolizeLabels(b, e.GetLabelsRefs(), symbols)
711
}
812

9-
func (t *TimeSeriesV2) ToLabels(b *labels.ScratchBuilder, symbols []string) labels.Labels {
13+
func (t *TimeSeriesV2) ToLabels(b *labels.ScratchBuilder, symbols []string) (labels.Labels, error) {
1014
return desymbolizeLabels(b, t.GetLabelsRefs(), symbols)
1115
}
1216

1317
// desymbolizeLabels decodes label references, with given symbols to labels.
14-
// Copied from the Prometheus: https://github.com/prometheus/prometheus/blob/v3.5.0/prompb/io/prometheus/write/v2/symbols.go#L76
15-
func desymbolizeLabels(b *labels.ScratchBuilder, labelRefs []uint32, symbols []string) labels.Labels {
18+
// Copied from the Prometheus: https://github.com/prometheus/prometheus/blob/v3.7.2/prompb/io/prometheus/write/v2/symbols.go#L83
19+
func desymbolizeLabels(b *labels.ScratchBuilder, labelRefs []uint32, symbols []string) (labels.Labels, error) {
20+
if len(labelRefs)%2 != 0 {
21+
return labels.EmptyLabels(), fmt.Errorf("invalid labelRefs length %d", len(labelRefs))
22+
}
23+
1624
b.Reset()
1725
for i := 0; i < len(labelRefs); i += 2 {
18-
b.Add(symbols[labelRefs[i]], symbols[labelRefs[i+1]])
26+
nameRef, valueRef := labelRefs[i], labelRefs[i+1]
27+
if int(nameRef) >= len(symbols) || int(valueRef) >= len(symbols) {
28+
return labels.EmptyLabels(), fmt.Errorf("labelRefs %d (name) = %d (value) outside of symbols table (size %d)", nameRef, valueRef, len(symbols))
29+
}
30+
b.Add(symbols[nameRef], symbols[valueRef])
1931
}
2032
b.Sort()
21-
return b.Labels()
33+
return b.Labels(), nil
2234
}

pkg/cortexpb/compatv2_test.go

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
package cortexpb
2+
3+
import (
4+
"testing"
5+
6+
"github.com/prometheus/prometheus/model/labels"
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
func Test_ExemplarV2ToLabels(t *testing.T) {
11+
symbols := []string{"", "foo", "bar"}
12+
b := &labels.ScratchBuilder{}
13+
14+
tests := []struct {
15+
desc string
16+
e *ExemplarV2
17+
isErr bool
18+
expectedLabels labels.Labels
19+
}{
20+
{
21+
desc: "Success",
22+
e: &ExemplarV2{
23+
LabelsRefs: []uint32{1, 2},
24+
},
25+
expectedLabels: labels.FromStrings("foo", "bar"),
26+
},
27+
{
28+
desc: "Odd length of the label refs",
29+
e: &ExemplarV2{
30+
LabelsRefs: []uint32{1},
31+
},
32+
isErr: true,
33+
},
34+
{
35+
desc: "Label name refs out of ranges",
36+
e: &ExemplarV2{
37+
LabelsRefs: []uint32{10, 2},
38+
},
39+
isErr: true,
40+
},
41+
{
42+
desc: "Label value refs out of ranges",
43+
e: &ExemplarV2{
44+
LabelsRefs: []uint32{1, 10},
45+
},
46+
isErr: true,
47+
},
48+
}
49+
50+
for _, test := range tests {
51+
t.Run(test.desc, func(t *testing.T) {
52+
lbs, err := test.e.ToLabels(b, symbols)
53+
if test.isErr {
54+
require.Error(t, err)
55+
} else {
56+
require.Equal(t, test.expectedLabels.String(), lbs.String())
57+
}
58+
})
59+
}
60+
}
61+
62+
func Test_TimeSeriesV2ToLabels(t *testing.T) {
63+
symbols := []string{"", "__name__", "test_metric", "job", "prometheus", "instance", "server-1"}
64+
b := &labels.ScratchBuilder{}
65+
66+
tests := []struct {
67+
desc string
68+
ts *TimeSeriesV2
69+
isErr bool
70+
expectedLabels labels.Labels
71+
}{
72+
{
73+
desc: "Success",
74+
ts: &TimeSeriesV2{
75+
LabelsRefs: []uint32{1, 2, 3, 4, 5, 6},
76+
},
77+
expectedLabels: labels.FromStrings("__name__", "test_metric", "job", "prometheus", "instance", "server-1"),
78+
},
79+
{
80+
desc: "Odd length of the label refs",
81+
ts: &TimeSeriesV2{
82+
LabelsRefs: []uint32{1, 2, 3},
83+
},
84+
isErr: true,
85+
},
86+
{
87+
desc: "Label name refs out of ranges",
88+
ts: &TimeSeriesV2{
89+
LabelsRefs: []uint32{1, 2, 10, 4},
90+
},
91+
isErr: true,
92+
},
93+
{
94+
desc: "Label value refs out of ranges",
95+
ts: &TimeSeriesV2{
96+
LabelsRefs: []uint32{1, 2, 3, 10},
97+
},
98+
isErr: true,
99+
},
100+
}
101+
102+
for _, test := range tests {
103+
t.Run(test.desc, func(t *testing.T) {
104+
lbs, err := test.ts.ToLabels(b, symbols)
105+
if test.isErr {
106+
require.Error(t, err)
107+
} else {
108+
require.Equal(t, test.expectedLabels.String(), lbs.String())
109+
}
110+
})
111+
}
112+
}

pkg/util/push/push.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,19 @@ func convertV2RequestToV1(req *cortexpb.PreallocWriteRequestV2) (cortexpb.Preall
177177
b := labels.NewScratchBuilder(0)
178178
symbols := req.Symbols
179179
for _, v2Ts := range req.Timeseries {
180-
lbs := v2Ts.ToLabels(&b, symbols)
180+
lbs, err := v2Ts.ToLabels(&b, symbols)
181+
if err != nil {
182+
return v1Req, err
183+
}
184+
exemplars, err := convertV2ToV1Exemplars(&b, symbols, v2Ts.Exemplars)
185+
if err != nil {
186+
return v1Req, err
187+
}
181188
v1Timeseries = append(v1Timeseries, cortexpb.PreallocTimeseries{
182189
TimeSeries: &cortexpb.TimeSeries{
183190
Labels: cortexpb.FromLabelsToLabelAdapters(lbs),
184191
Samples: v2Ts.Samples,
185-
Exemplars: convertV2ToV1Exemplars(&b, symbols, v2Ts.Exemplars),
192+
Exemplars: exemplars,
186193
Histograms: v2Ts.Histograms,
187194
},
188195
})
@@ -234,14 +241,18 @@ func convertV2ToV1Metadata(name string, symbols []string, metadata cortexpb.Meta
234241
}
235242
}
236243

237-
func convertV2ToV1Exemplars(b *labels.ScratchBuilder, symbols []string, v2Exemplars []cortexpb.ExemplarV2) []cortexpb.Exemplar {
244+
func convertV2ToV1Exemplars(b *labels.ScratchBuilder, symbols []string, v2Exemplars []cortexpb.ExemplarV2) ([]cortexpb.Exemplar, error) {
238245
v1Exemplars := make([]cortexpb.Exemplar, 0, len(v2Exemplars))
239246
for _, e := range v2Exemplars {
247+
lbs, err := e.ToLabels(b, symbols)
248+
if err != nil {
249+
return nil, err
250+
}
240251
v1Exemplars = append(v1Exemplars, cortexpb.Exemplar{
241-
Labels: cortexpb.FromLabelsToLabelAdapters(e.ToLabels(b, symbols)),
252+
Labels: cortexpb.FromLabelsToLabelAdapters(lbs),
242253
Value: e.Value,
243254
TimestampMs: e.Timestamp,
244255
})
245256
}
246-
return v1Exemplars
257+
return v1Exemplars, nil
247258
}

0 commit comments

Comments
 (0)